fix(app): simplify review layout (#14953)

Co-authored-by: adamelmore <[email protected]>

2 files changed, 111 insertions, 141 deletions

diff --git a/packages/app/src/pages/session.tsx b/packages/app/src/pages/session.tsx
index 2e440a6b0..6751f4186 100644
--- a/packages/app/src/pages/session.tsx
+++ b/packages/app/src/pages/session.tsx
@@ -416,7 +416,7 @@ export default function Page() {
   )
 
   const mobileChanges = createMemo(() => !isDesktop() && store.mobileTab === "changes")
-  const reviewTab = createMemo(() => isDesktop() && !layout.fileTree.opened())
+  const reviewTab = createMemo(() => isDesktop())
 
   const fileTreeTab = () => layout.fileTree.tab()
   const setFileTreeTab = (value: "changes" | "all") => layout.fileTree.setTab(value)
@@ -700,34 +700,13 @@ export default function Page() {
           const active = tabs().active()
           const tab = active === "review" || (!active && hasReview()) ? "changes" : "all"
           layout.fileTree.setTab(tab)
-          return
         }
-
-        if (fileTreeTab() !== "changes") return
-        tabs().setActive("review")
       },
       { defer: true },
     ),
   )
 
   createEffect(() => {
-    if (!isDesktop()) return
-    if (!layout.fileTree.opened()) return
-    if (fileTreeTab() !== "all") return
-
-    const active = tabs().active()
-    if (active && active !== "review") return
-
-    const first = openedTabs()[0]
-    if (first) {
-      tabs().setActive(first)
-      return
-    }
-
-    if (contextOpen()) tabs().setActive("context")
-  })
-
-  createEffect(() => {
     const id = params.id
     if (!id) return
 
diff --git a/packages/app/src/pages/session/session-side-panel.tsx b/packages/app/src/pages/session/session-side-panel.tsx
index 07b18f314..efcc7c95c 100644
--- a/packages/app/src/pages/session/session-side-panel.tsx
+++ b/packages/app/src/pages/session/session-side-panel.tsx
@@ -47,7 +47,7 @@ export function SessionSidePanel(props: {
 
   const reviewOpen = createMemo(() => isDesktop() && view().reviewPanel.opened())
   const open = createMemo(() => isDesktop() && (view().reviewPanel.opened() || layout.fileTree.opened()))
-  const reviewTab = createMemo(() => isDesktop() && !layout.fileTree.opened())
+  const reviewTab = createMemo(() => isDesktop())
 
   const info = createMemo(() => (params.id ? sync.session.get(params.id) : undefined))
   const diffs = createMemo(() => (params.id ? (sync.data.session_diff[params.id] ?? []) : []))
@@ -202,133 +202,124 @@ export function SessionSidePanel(props: {
       >
         <Show when={reviewOpen()}>
           <div class="flex-1 min-w-0 h-full">
-            <Show
-              when={layout.fileTree.opened() && fileTreeTab() === "changes"}
-              fallback={
-                <DragDropProvider
-                  onDragStart={handleDragStart}
-                  onDragEnd={handleDragEnd}
-                  onDragOver={handleDragOver}
-                  collisionDetector={closestCenter}
-                >
-                  <DragDropSensors />
-                  <ConstrainDragYAxis />
-                  <Tabs value={activeTab()} onChange={openTab}>
-                    <div class="sticky top-0 shrink-0 flex">
-                      <Tabs.List
-                        ref={(el: HTMLDivElement) => {
-                          const stop = createFileTabListSync({ el, contextOpen })
-                          onCleanup(stop)
-                        }}
-                      >
-                        <Show when={reviewTab()}>
-                          <Tabs.Trigger value="review" classes={{ button: "!pl-6" }}>
-                            <div class="flex items-center gap-1.5">
-                              <div>{language.t("session.tab.review")}</div>
-                              <Show when={hasReview()}>
-                                <div class="text-12-medium text-text-strong h-4 px-2 flex flex-col items-center justify-center rounded-full bg-surface-base">
-                                  {reviewCount()}
-                                </div>
-                              </Show>
-                            </div>
-                          </Tabs.Trigger>
-                        </Show>
-                        <Show when={contextOpen()}>
-                          <Tabs.Trigger
-                            value="context"
-                            closeButton={
-                              <Tooltip value={language.t("common.closeTab")} placement="bottom">
-                                <IconButton
-                                  icon="close-small"
-                                  variant="ghost"
-                                  class="h-5 w-5"
-                                  onClick={() => tabs().close("context")}
-                                  aria-label={language.t("common.closeTab")}
-                                />
-                              </Tooltip>
-                            }
-                            hideCloseButton
-                            onMiddleClick={() => tabs().close("context")}
-                          >
-                            <div class="flex items-center gap-2">
-                              <SessionContextUsage variant="indicator" />
-                              <div>{language.t("session.tab.context")}</div>
+            <DragDropProvider
+              onDragStart={handleDragStart}
+              onDragEnd={handleDragEnd}
+              onDragOver={handleDragOver}
+              collisionDetector={closestCenter}
+            >
+              <DragDropSensors />
+              <ConstrainDragYAxis />
+              <Tabs value={activeTab()} onChange={openTab}>
+                <div class="sticky top-0 shrink-0 flex">
+                  <Tabs.List
+                    ref={(el: HTMLDivElement) => {
+                      const stop = createFileTabListSync({ el, contextOpen })
+                      onCleanup(stop)
+                    }}
+                  >
+                    <Show when={reviewTab()}>
+                      <Tabs.Trigger value="review" classes={{ button: "!pl-6" }}>
+                        <div class="flex items-center gap-1.5">
+                          <div>{language.t("session.tab.review")}</div>
+                          <Show when={hasReview()}>
+                            <div class="text-12-medium text-text-strong h-4 px-2 flex flex-col items-center justify-center rounded-full bg-surface-base">
+                              {reviewCount()}
                             </div>
-                          </Tabs.Trigger>
-                        </Show>
-                        <SortableProvider ids={openedTabs()}>
-                          <For each={openedTabs()}>{(tab) => <SortableTab tab={tab} onTabClose={tabs().close} />}</For>
-                        </SortableProvider>
-                        <StickyAddButton>
-                          <TooltipKeybind
-                            title={language.t("command.file.open")}
-                            keybind={command.keybind("file.open")}
-                            class="flex items-center"
-                          >
+                          </Show>
+                        </div>
+                      </Tabs.Trigger>
+                    </Show>
+                    <Show when={contextOpen()}>
+                      <Tabs.Trigger
+                        value="context"
+                        closeButton={
+                          <Tooltip value={language.t("common.closeTab")} placement="bottom">
                             <IconButton
-                              icon="plus-small"
+                              icon="close-small"
                               variant="ghost"
-                              iconSize="large"
-                              onClick={() =>
-                                dialog.show(() => <DialogSelectFile mode="files" onOpenFile={showAllFiles} />)
-                              }
-                              aria-label={language.t("command.file.open")}
+                              class="h-5 w-5"
+                              onClick={() => tabs().close("context")}
+                              aria-label={language.t("common.closeTab")}
                             />
-                          </TooltipKeybind>
-                        </StickyAddButton>
-                      </Tabs.List>
-                    </div>
-
-                    <Show when={reviewTab()}>
-                      <Tabs.Content value="review" class="flex flex-col h-full overflow-hidden contain-strict">
-                        <Show when={activeTab() === "review"}>{props.reviewPanel()}</Show>
-                      </Tabs.Content>
+                          </Tooltip>
+                        }
+                        hideCloseButton
+                        onMiddleClick={() => tabs().close("context")}
+                      >
+                        <div class="flex items-center gap-2">
+                          <SessionContextUsage variant="indicator" />
+                          <div>{language.t("session.tab.context")}</div>
+                        </div>
+                      </Tabs.Trigger>
                     </Show>
-
-                    <Tabs.Content value="empty" class="flex flex-col h-full overflow-hidden contain-strict">
-                      <Show when={activeTab() === "empty"}>
-                        <div class="relative pt-2 flex-1 min-h-0 overflow-hidden">
-                          <div class="h-full px-6 pb-42 flex flex-col items-center justify-center text-center gap-6">
-                            <Mark class="w-14 opacity-10" />
-                            <div class="text-14-regular text-text-weak max-w-56">
-                              {language.t("session.files.selectToOpen")}
-                            </div>
-                          </div>
+                    <SortableProvider ids={openedTabs()}>
+                      <For each={openedTabs()}>{(tab) => <SortableTab tab={tab} onTabClose={tabs().close} />}</For>
+                    </SortableProvider>
+                    <StickyAddButton>
+                      <TooltipKeybind
+                        title={language.t("command.file.open")}
+                        keybind={command.keybind("file.open")}
+                        class="flex items-center"
+                      >
+                        <IconButton
+                          icon="plus-small"
+                          variant="ghost"
+                          iconSize="large"
+                          onClick={() => dialog.show(() => <DialogSelectFile mode="files" onOpenFile={showAllFiles} />)}
+                          aria-label={language.t("command.file.open")}
+                        />
+                      </TooltipKeybind>
+                    </StickyAddButton>
+                  </Tabs.List>
+                </div>
+
+                <Show when={reviewTab()}>
+                  <Tabs.Content value="review" class="flex flex-col h-full overflow-hidden contain-strict">
+                    <Show when={activeTab() === "review"}>{props.reviewPanel()}</Show>
+                  </Tabs.Content>
+                </Show>
+
+                <Tabs.Content value="empty" class="flex flex-col h-full overflow-hidden contain-strict">
+                  <Show when={activeTab() === "empty"}>
+                    <div class="relative pt-2 flex-1 min-h-0 overflow-hidden">
+                      <div class="h-full px-6 pb-42 flex flex-col items-center justify-center text-center gap-6">
+                        <Mark class="w-14 opacity-10" />
+                        <div class="text-14-regular text-text-weak max-w-56">
+                          {language.t("session.files.selectToOpen")}
                         </div>
-                      </Show>
-                    </Tabs.Content>
+                      </div>
+                    </div>
+                  </Show>
+                </Tabs.Content>
 
-                    <Show when={contextOpen()}>
-                      <Tabs.Content value="context" class="flex flex-col h-full overflow-hidden contain-strict">
-                        <Show when={activeTab() === "context"}>
-                          <div class="relative pt-2 flex-1 min-h-0 overflow-hidden">
-                            <SessionContextTab />
-                          </div>
-                        </Show>
-                      </Tabs.Content>
+                <Show when={contextOpen()}>
+                  <Tabs.Content value="context" class="flex flex-col h-full overflow-hidden contain-strict">
+                    <Show when={activeTab() === "context"}>
+                      <div class="relative pt-2 flex-1 min-h-0 overflow-hidden">
+                        <SessionContextTab />
+                      </div>
                     </Show>
+                  </Tabs.Content>
+                </Show>
 
-                    <Show when={activeFileTab()} keyed>
-                      {(tab) => <FileTabContent tab={tab} />}
-                    </Show>
-                  </Tabs>
-                  <DragOverlay>
-                    <Show when={store.activeDraggable} keyed>
-                      {(tab) => {
-                        const path = createMemo(() => file.pathFromTab(tab))
-                        return (
-                          <div class="relative px-6 h-12 flex items-center bg-background-stronger border-x border-border-weak-base border-b border-b-transparent">
-                            <Show when={path()}>{(p) => <FileVisual active path={p()} />}</Show>
-                          </div>
-                        )
-                      }}
-                    </Show>
-                  </DragOverlay>
-                </DragDropProvider>
-              }
-            >
-              {props.reviewPanel()}
-            </Show>
+                <Show when={activeFileTab()} keyed>
+                  {(tab) => <FileTabContent tab={tab} />}
+                </Show>
+              </Tabs>
+              <DragOverlay>
+                <Show when={store.activeDraggable} keyed>
+                  {(tab) => {
+                    const path = createMemo(() => file.pathFromTab(tab))
+                    return (
+                      <div class="relative px-6 h-12 flex items-center bg-background-stronger border-x border-border-weak-base border-b border-b-transparent">
+                        <Show when={path()}>{(p) => <FileVisual active path={p()} />}</Show>
+                      </div>
+                    )
+                  }}
+                </Show>
+              </DragOverlay>
+            </DragDropProvider>
           </div>
         </Show>