Kaynağa Gözat

fix(app): follow visual tab order (#39241)

Brendan Allan 1 ay önce
ebeveyn
işleme
87db7a7f0a

+ 33 - 1
packages/app/e2e/regression/tab-navigate-mousedown.spec.ts

@@ -5,6 +5,8 @@ import { currentSession } from "../utils/mock-server"
 const server = "http://127.0.0.1:4096"
 const sessionA = session("ses_tab_a", "Tab A session")
 const sessionB = session("ses_tab_b", "Tab B session")
+const sessionC = session("ses_tab_c", "Tab C session")
+const unresolvedSessionID = "ses_tab_unresolved"
 
 test("pressing mouse down on a tab navigates before mouse up", async ({ page }) => {
   await mockServer(page)
@@ -40,6 +42,34 @@ test("pressing mouse down on a tab navigates before mouse up", async ({ page })
   await expect(page).toHaveURL(new RegExp(`${hrefB.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}$`))
 })
 
+test("keyboard navigation follows the visible tab order", async ({ page }) => {
+  await mockServer(page)
+  await page.addInitScript(
+    ({ server, sessionA, unresolved, sessionC }) => {
+      localStorage.setItem("settings.v3", JSON.stringify({ general: { newLayoutDesigns: true } }))
+      localStorage.setItem(
+        "opencode.window.browser.dat:tabs",
+        JSON.stringify([
+          { type: "session", server, sessionId: sessionA },
+          { type: "session", server, sessionId: unresolved },
+          { type: "session", server, sessionId: sessionC },
+        ]),
+      )
+    },
+    { server, sessionA: sessionA.id, unresolved: unresolvedSessionID, sessionC: sessionC.id },
+  )
+
+  const hrefA = `/server/${base64Encode(server)}/session/${sessionA.id}`
+  const hrefC = `/server/${base64Encode(server)}/session/${sessionC.id}`
+  await page.goto(hrefA)
+  await expect(page.locator("[data-titlebar-tab-slot]:visible")).toHaveCount(2)
+  await expect(page.locator(`[data-titlebar-tab-slot]:has(a[href="${hrefC}"])`)).toBeVisible()
+
+  await page.keyboard.press("Control+Alt+ArrowRight")
+
+  await expect(page).toHaveURL(new RegExp(`${hrefC.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}$`))
+})
+
 function session(id: string, title: string) {
   return {
     id,
@@ -53,10 +83,12 @@ function session(id: string, title: string) {
 }
 
 async function mockServer(page: Page) {
-  const sessions = [sessionA, sessionB]
+  const sessions = [sessionA, sessionB, sessionC]
   await page.route("**/*", async (route) => {
     const url = new URL(route.request().url())
     if (url.origin !== server) return route.fallback()
+    if ([`/api/session/${unresolvedSessionID}`, `/session/${unresolvedSessionID}`].includes(url.pathname))
+      return new Promise(() => {})
     if (url.pathname === "/global/event" || url.pathname === "/event" || url.pathname === "/api/event")
       return sse(route)
     if (url.pathname === "/global/health") return json(route, { healthy: true })

+ 74 - 76
packages/app/src/components/titlebar-tab-nav.tsx

@@ -195,88 +195,86 @@ export function TabNavItem(props: {
         closeTab(event)
       }}
     >
-      <Show when={title() !== undefined}>
-        <a
-          data-slot="tab-link"
-          data-titlebar-tab-link
-          href={props.href}
-          draggable={false}
-          onDragStart={(event) => {
-            event.preventDefault()
+      <a
+        data-slot="tab-link"
+        data-titlebar-tab-link
+        href={props.href}
+        draggable={false}
+        onDragStart={(event) => {
+          event.preventDefault()
+          event.stopPropagation()
+        }}
+        onMouseDown={(event) => {
+          // Navigate on mousedown to shave the press-release delay off tab switches.
+          if (event.button !== 0) return
+          if (editing()) return
+          if (props.suppressNavigation?.()) return
+          props.onNavigate()
+        }}
+        onClick={(event) => {
+          event.preventDefault()
+          // Mouse navigation already happened on mousedown; detail 0 means keyboard activation.
+          if (event.detail > 0) return
+          if (editing()) return
+          if (props.suppressNavigation?.()) return
+          props.onNavigate()
+        }}
+        class="flex h-full min-w-0 flex-1 flex-row items-center gap-1.5 text-[13px] font-medium text-v2-text-text-faint group-data-[active='true']:text-v2-text-text-base group-data-[editing='true']:text-v2-text-text-base [-webkit-user-drag:none]"
+      >
+        <span data-slot="project-avatar-slot" class="flex size-4 shrink-0 items-center justify-center">
+          <Show
+            when={props.session()}
+            fallback={
+              <span class="block size-4 rounded-[3px] border border-v2-border-border-muted" aria-hidden="true" />
+            }
+          >
+            {(session) => (
+              <SessionTabAvatar
+                project={project()}
+                directory={session().directory}
+                sessionId={session().id}
+                server={props.server}
+              />
+            )}
+          </Show>
+        </span>
+        <span
+          ref={(el) => {
+            titleEl = el
+            titleEl.textContent = title() ?? ""
+          }}
+          data-slot="tab-title"
+          data-titlebar-tab-title
+          class="min-w-0 flex-1 outline-none leading-4"
+          classList={{
+            "overflow-hidden text-clip whitespace-nowrap": !editing(),
+            "select-text": editing(),
+          }}
+          contenteditable={editing() ? true : undefined}
+          onDblClick={openRename}
+          onKeyDown={(event) => {
             event.stopPropagation()
+            if (event.key === "Enter") {
+              event.preventDefault()
+              void closeRename(true)
+              return
+            }
+            if (event.key !== "Escape") return
+            event.preventDefault()
+            titleEl.textContent = props.session()?.title ?? ""
+            void closeRename(false)
           }}
-          onMouseDown={(event) => {
-            // Navigate on mousedown to shave the press-release delay off tab switches.
-            if (event.button !== 0) return
-            if (editing()) return
-            if (props.suppressNavigation?.()) return
-            props.onNavigate()
+          onBlur={() => void closeRename(true)}
+          onPointerDown={(event) => {
+            if (!editing()) return
+            event.stopPropagation()
           }}
           onClick={(event) => {
+            if (!editing()) return
             event.preventDefault()
-            // Mouse navigation already happened on mousedown; detail 0 means keyboard activation.
-            if (event.detail > 0) return
-            if (editing()) return
-            if (props.suppressNavigation?.()) return
-            props.onNavigate()
           }}
-          class="flex h-full min-w-0 flex-1 flex-row items-center gap-1.5 text-[13px] font-medium text-v2-text-text-faint group-data-[active='true']:text-v2-text-text-base group-data-[editing='true']:text-v2-text-text-base [-webkit-user-drag:none]"
-        >
-          <span data-slot="project-avatar-slot" class="flex size-4 shrink-0 items-center justify-center">
-            <Show
-              when={props.session()}
-              fallback={
-                <span class="block size-4 rounded-[3px] border border-v2-border-border-muted" aria-hidden="true" />
-              }
-            >
-              {(session) => (
-                <SessionTabAvatar
-                  project={project()}
-                  directory={session().directory}
-                  sessionId={session().id}
-                  server={props.server}
-                />
-              )}
-            </Show>
-          </span>
-          <span
-            ref={(el) => {
-              titleEl = el
-              titleEl.textContent = title() ?? ""
-            }}
-            data-slot="tab-title"
-            data-titlebar-tab-title
-            class="min-w-0 flex-1 outline-none leading-4"
-            classList={{
-              "overflow-hidden text-clip whitespace-nowrap": !editing(),
-              "select-text": editing(),
-            }}
-            contenteditable={editing() ? true : undefined}
-            onDblClick={openRename}
-            onKeyDown={(event) => {
-              event.stopPropagation()
-              if (event.key === "Enter") {
-                event.preventDefault()
-                void closeRename(true)
-                return
-              }
-              if (event.key !== "Escape") return
-              event.preventDefault()
-              titleEl.textContent = props.session()?.title ?? ""
-              void closeRename(false)
-            }}
-            onBlur={() => void closeRename(true)}
-            onPointerDown={(event) => {
-              if (!editing()) return
-              event.stopPropagation()
-            }}
-            onClick={(event) => {
-              if (!editing()) return
-              event.preventDefault()
-            }}
-          />
-        </a>
-      </Show>
+        />
+      </a>
 
       <div data-slot="tab-close" class="group-hover:bg-[var(--tab-bg)] group-data-[active=true]:bg-[var(--tab-bg)]">
         <IconButtonV2

+ 23 - 0
packages/app/src/components/titlebar-tab-order.test.ts

@@ -0,0 +1,23 @@
+import { describe, expect, test } from "bun:test"
+import { adjacentTabKey, mergeVisibleTabOrder } from "./titlebar-tab-order"
+
+describe("adjacentTabKey", () => {
+  test("follows the visible left-to-right order", () => {
+    expect(adjacentTabKey(["c", "a", "b"], "c", 1)).toBe("a")
+    expect(adjacentTabKey(["c", "a", "b"], "a", -1)).toBe("c")
+  })
+
+  test("skips tabs omitted from the visible order", () => {
+    expect(adjacentTabKey(["a", "c"], "a", 1)).toBe("c")
+    expect(adjacentTabKey(["a", "c"], "c", 1)).toBe("a")
+  })
+})
+
+test("merges reordered visible tabs around hidden tabs", () => {
+  expect(mergeVisibleTabOrder(["a", "hidden", "b", "c"], ["a", "b", "c"], ["c", "a", "b"])).toEqual([
+    "c",
+    "hidden",
+    "a",
+    "b",
+  ])
+})

+ 12 - 0
packages/app/src/components/titlebar-tab-order.ts

@@ -0,0 +1,12 @@
+export function adjacentTabKey(order: string[], current: string | undefined, offset: -1 | 1) {
+  if (!current || order.length === 0) return
+  const index = order.indexOf(current)
+  if (index === -1) return
+  return order[(index + offset + order.length) % order.length]
+}
+
+export function mergeVisibleTabOrder(all: string[], current: string[], next: string[]) {
+  const visible = new Set(current)
+  const reordered = next.values()
+  return all.map((key) => (visible.has(key) ? (reordered.next().value ?? key) : key))
+}

+ 106 - 33
packages/app/src/components/titlebar-tab-strip.tsx

@@ -1,4 +1,5 @@
-import { createEffect, createMemo, createResource, createRoot, For, onCleanup, onMount } from "solid-js"
+import { createEffect, createMemo, createResource, createRoot, For, onCleanup, onMount, Show } from "solid-js"
+import { createStore } from "solid-js/store"
 import { createResizeObserver } from "@solid-primitives/resize-observer"
 import { DragDropProvider, PointerSensor } from "@dnd-kit/solid"
 import { isSortable, useSortable } from "@dnd-kit/solid/sortable"
@@ -17,6 +18,8 @@ import { createTabPromptState } from "@/context/prompt"
 import { base64Encode } from "@opencode-ai/core/util/encode"
 import { showToast } from "@/utils/toast"
 import { canStartTabDrag, isTabCloseTarget } from "./titlebar-tab-gesture"
+import { adjacentTabKey, mergeVisibleTabOrder } from "./titlebar-tab-order"
+import type { Session } from "@opencode-ai/sdk/v2"
 
 function SessionTabSlot(props: {
   tab: SessionTab
@@ -24,12 +27,12 @@ function SessionTabSlot(props: {
   index: () => number
   active: () => boolean
   forceTruncate: boolean
-  serverCtx: () => ServerCtx | undefined
+  session: () => Session | undefined
+  fallbackTitle?: string
+  onRename: (title: string) => Promise<void>
   onNavigate: (element: HTMLDivElement) => void
   onClose: () => void
 }) {
-  const tabs = useTabs()
-  const language = useLanguage()
   const sortable = useSortable({
     get id() {
       return props.id
@@ -39,6 +42,47 @@ function SessionTabSlot(props: {
     },
   })
   let ref!: HTMLDivElement
+
+  return (
+    <div
+      ref={sortable.ref}
+      data-titlebar-tab-slot
+      data-tab-key={props.id}
+      data-active={props.active()}
+      class="relative flex w-56 min-w-7 max-w-56 flex-shrink"
+    >
+      <TabNavItem
+        ref={(el) => {
+          ref = el
+        }}
+        href={tabHref(props.tab)}
+        server={props.tab.server}
+        session={props.session}
+        fallbackTitle={props.fallbackTitle}
+        onRename={props.onRename}
+        onNavigate={() => props.onNavigate(ref)}
+        onClose={props.onClose}
+        active={props.active()}
+        forceTruncate={props.forceTruncate}
+        dragging={sortable.isDragSource()}
+      />
+    </div>
+  )
+}
+
+function SessionTabEntry(props: {
+  tab: SessionTab
+  id: string
+  index: () => number
+  active: () => boolean
+  forceTruncate: boolean
+  serverCtx: () => ServerCtx | undefined
+  onVisibleChange: (visible: boolean) => void
+  onNavigate: (element: HTMLDivElement) => void
+  onClose: () => void
+}) {
+  const tabs = useTabs()
+  const language = useLanguage()
   const sdk = createMemo(() => props.serverCtx()?.sdk ?? null)
   const cachedSession = createMemo(() => props.serverCtx()?.sync.session.peek(props.tab.sessionId))
   const persisted = createMemo(() => tabs.info[props.id])
@@ -51,6 +95,7 @@ function SessionTabSlot(props: {
   )
   const session = createMemo(() => cachedSession() ?? loadedSession())
   const missingSession = createMemo(() => !!props.serverCtx() && !loadedSession.loading && !session())
+  const visible = createMemo(() => !!session() || missingSession() || !!persisted()?.title)
   let prefetched = false
 
   const rename = async (title: string) => {
@@ -72,6 +117,8 @@ function SessionTabSlot(props: {
     }
   }
 
+  createEffect(() => props.onVisibleChange(visible()))
+
   createEffect(() => {
     const ctx = props.serverCtx()
     const value = session()
@@ -103,30 +150,20 @@ function SessionTabSlot(props: {
   })
 
   return (
-    <div
-      ref={sortable.ref}
-      data-titlebar-tab-slot
-      data-tab-key={props.id}
-      data-active={props.active()}
-      class="relative flex w-56 min-w-7 max-w-56 flex-shrink"
-      classList={{ hidden: !session() && !missingSession() && !persisted()?.title }}
-    >
-      <TabNavItem
-        ref={(el) => {
-          ref = el
-        }}
-        href={tabHref(props.tab)}
-        server={props.tab.server}
+    <Show when={visible()}>
+      <SessionTabSlot
+        tab={props.tab}
+        id={props.id}
+        index={props.index}
+        active={props.active}
+        forceTruncate={props.forceTruncate}
         session={session}
         fallbackTitle={persisted()?.title ?? (missingSession() ? language.t("session.tab.unknown") : undefined)}
         onRename={rename}
-        onNavigate={() => props.onNavigate(ref)}
+        onNavigate={props.onNavigate}
         onClose={props.onClose}
-        active={props.active()}
-        forceTruncate={props.forceTruncate}
-        dragging={sortable.isDragSource()}
       />
-    </div>
+    </Show>
   )
 }
 
@@ -183,11 +220,39 @@ export function TitlebarTabStrip(props: {
 }) {
   const global = useGlobal()
   const language = useLanguage()
+  const command = useCommand()
   let scrollRef!: HTMLDivElement
   let listRef!: HTMLDivElement
   let resizeFrame: number | undefined
+  const [visibility, setVisibility] = createStore<Record<string, boolean>>({})
+  const visibleTabs = createMemo(() => props.tabs.filter((tab) => tab.type === "draft" || visibility[tabKey(tab)]))
+  const visibleTabIds = () => visibleTabs().map(tabKey)
 
-  const tabIds = () => props.tabs.map(tabKey)
+  command.register("titlebar-tab-cycle", () => [
+    {
+      id: `tab.prev`,
+      category: "tab",
+      title: "",
+      keybind: `mod+option+ArrowLeft,ctrl+shift+tab`,
+      hidden: true,
+      onSelect: () => selectAdjacentTab(-1),
+    },
+    {
+      id: `tab.next`,
+      category: "tab",
+      title: "",
+      keybind: `mod+option+ArrowRight,ctrl+tab`,
+      hidden: true,
+      onSelect: () => selectAdjacentTab(1),
+    },
+  ])
+
+  function selectAdjacentTab(offset: -1 | 1) {
+    const current = props.currentTab()
+    const key = adjacentTabKey(visibleTabIds(), current ? tabKey(current) : undefined, offset)
+    const next = props.tabs.find((tab) => tabKey(tab) === key)
+    if (next) props.onNavigate(next)
+  }
 
   function refreshOverflow() {
     if (!scrollRef) return
@@ -215,7 +280,7 @@ export function TitlebarTabStrip(props: {
 
   createEffect(() => {
     props.tabs.length
-    tabIds()
+    visibleTabIds()
     refreshOverflow()
   })
 
@@ -251,22 +316,29 @@ export function TitlebarTabStrip(props: {
             props.onNavigate(tab, tabEl ?? undefined)
           }}
           onDragEnd={(event) => {
-            const current = tabIds()
+            const current = visibleTabIds()
             const source = event.operation.source
             if (event.canceled || !isSortable(source)) return
 
             const { initialIndex, index } = source
             if (initialIndex !== index) {
-              props.onReorder(arrayMove(current, source.initialIndex, source.index))
+              props.onReorder(
+                mergeVisibleTabOrder(
+                  props.tabs.map(tabKey),
+                  current,
+                  arrayMove(current, source.initialIndex, source.index),
+                ),
+              )
             }
           }}
         >
           <div data-titlebar-tab-list class="flex w-full min-w-0 flex-row items-center" ref={listRef}>
             <For each={props.tabs}>
-              {(tab, index) => {
+              {(tab) => {
                 const id = tabKey(tab)
                 let ref!: HTMLDivElement
-                useTabShortcut(index, () => props.onNavigate(tab, ref))
+                const visibleIndex = () => visibleTabs().findIndex((item) => tabKey(item) === id)
+                useTabShortcut(visibleIndex, () => props.onNavigate(tab, ref))
                 const serverCtx = createMemo(() => {
                   if (tab.type !== "session") return
                   const conn = global.servers.list().find((item) => ServerConnection.key(item) === tab.server)
@@ -275,13 +347,14 @@ export function TitlebarTabStrip(props: {
 
                 if (tab.type === "session") {
                   return (
-                    <SessionTabSlot
+                    <SessionTabEntry
                       tab={tab}
                       id={id}
-                      index={index}
+                      index={visibleIndex}
                       active={() => props.currentTab() === tab}
                       forceTruncate={props.forceTruncate}
                       serverCtx={serverCtx}
+                      onVisibleChange={(visible) => setVisibility(id, visible)}
                       onNavigate={(element) => {
                         ref = element
                         props.onNavigate(tab, element)
@@ -295,7 +368,7 @@ export function TitlebarTabStrip(props: {
                   <DraftTabSlot
                     tab={tab}
                     id={id}
-                    index={index}
+                    index={visibleIndex}
                     active={() => props.currentTab() === tab}
                     title={language.t("command.session.new")}
                     onNavigate={(element) => {
@@ -329,7 +402,7 @@ function useTabShortcut(index: () => number, onSelect: () => void) {
 
   command.register(() => {
     const number = index() + 1
-    if (number > 9) return []
+    if (number < 1 || number > 9) return []
     return [
       {
         id: `tab.${number}`,

+ 0 - 34
packages/app/src/components/titlebar.tsx

@@ -338,40 +338,6 @@ export function Titlebar(props: { update?: TitlebarUpdate; debugTools?: { visibl
                   keybind: "mod+shift+t",
                   onSelect: () => tabsStoreActions.reopenClosedTab(),
                 },
-                {
-                  id: `tab.prev`,
-                  category: "tab",
-                  title: "",
-                  keybind: `mod+option+ArrowLeft,ctrl+shift+tab`,
-                  hidden: true,
-                  onSelect: () => {
-                    let index = tabsStore.findIndex((tab) => tab === currentTab())
-                    if (index === -1) return
-
-                    index -= 1
-                    if (index === -1) index = tabsStore.length - 1
-
-                    const next = tabsStore[index]
-                    if (next) tabs.select(next)
-                  },
-                },
-                {
-                  id: `tab.next`,
-                  category: "tab",
-                  title: "",
-                  keybind: `mod+option+ArrowRight,ctrl+tab`,
-                  hidden: true,
-                  onSelect: () => {
-                    let index = tabsStore.findIndex((tab) => tab === currentTab())
-                    if (index === -1) return
-
-                    index += 1
-                    if (index === tabsStore.length) index = 0
-
-                    const next = tabsStore[index]
-                    if (next) tabs.select(next)
-                  },
-                },
               ].filter((v) => v !== undefined)
             })