From b9fb87a3ab22564555d15a10d9f6b6ad47ed46b6 Mon Sep 17 00:00:00 2001 From: Harry19081 <20519290+Harry19081@users.noreply.github.com> Date: Wed, 26 Aug 2026 09:11:24 +0800 Subject: [PATCH] fix(sidebar): hide branch tags by default --- .../__tests__/menuItemBuilders.test.ts | 108 +++++++++++++----- .../connectors/useSessionMenuItems/index.tsx | 21 +++- .../useSessionMenuItems/menuItemBuilders.tsx | 15 ++- .../useSessionPrStatuses.ts | 2 +- src/store/ui/__tests__/sidebarAtom.test.ts | 40 +++++++ src/store/ui/sidebarAtom.ts | 21 ++++ 6 files changed, 164 insertions(+), 43 deletions(-) diff --git a/src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/__tests__/menuItemBuilders.test.ts b/src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/__tests__/menuItemBuilders.test.ts index 31c1e12c91..c7d9661129 100644 --- a/src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/__tests__/menuItemBuilders.test.ts +++ b/src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/__tests__/menuItemBuilders.test.ts @@ -18,13 +18,13 @@ const BASE_SESSION = { function buildItem( session: Partial, visited = ["s1"], - pr?: BranchPrSnapshot + options: { showBranchTag?: boolean; pr?: BranchPrSnapshot } = {} ) { return buildSessionMenuItem({ session: { ...BASE_SESSION, ...session }, untitledSession: "Untitled", visitedSessions: new Set(visited), - pr, + ...options, }); } @@ -39,15 +39,53 @@ describe("buildSessionMenuItem trailing accessories", () => { expect(html).not.toContain("aria-label"); }); - it("puts the git indicator before the status dot", () => { + it.each([ + ["branch", { branch: "main" }], + ["worktree branch", { worktreeBranch: "agent/feature-x" }], + ])("does not render a %s tag", (_label, session) => { + const html = markup(buildItem(session).trailingElement); + expect(html).toContain("rounded-full"); + expect(html).not.toContain("aria-label"); + }); + + it("does not render a PR tag when the preference is disabled", () => { + const html = markup( + buildItem({ branch: "main" }, ["s1"], { + pr: { + status: "open", + number: 1, + url: "https://github.com/o/r/pull/1", + title: "Add thing", + }, + }).trailingElement + ); + + expect(html).toContain("rounded-full"); + expect(html).not.toContain("Open PR"); + }); + + it("does not leave a trailing branch tag on a working row", () => { + const item = buildItem( + { status: "running", worktreeBranch: "agent/feature-x" }, + [] + ); + expect(item.trailingElement).toBeUndefined(); + expect(markup(item.workingIndicator)).toContain('aria-label="Working"'); + }); + + it("puts the enabled git indicator before the status dot", () => { const html = markup( buildItem({ branch: "main" }, ["s1"], { - status: "open", - number: 1, - url: "https://github.com/o/r/pull/1", - title: "Add thing", + showBranchTag: true, + pr: { + status: "open", + number: 1, + url: "https://github.com/o/r/pull/1", + title: "Add thing", + }, }).trailingElement ); + const gitIndex = html.indexOf('aria-label="Open PR #1: main"'); const dotIndex = html.indexOf("rounded-full"); expect(gitIndex).toBeGreaterThanOrEqual(0); @@ -55,28 +93,31 @@ describe("buildSessionMenuItem trailing accessories", () => { expect(gitIndex).toBeLessThan(dotIndex); }); - it("shows no marker for a branch with no pull request", () => { - // The generic branch glyph is deliberately gone: it could not distinguish - // "no PR", "not fetched yet", "not GitHub", and "unknown state". - const html = markup(buildItem({ branch: "main" }).trailingElement); + it("shows no enabled marker for a branch with no pull request", () => { + const html = markup( + buildItem({ branch: "main" }, ["s1"], { showBranchTag: true }) + .trailingElement + ); expect(html).toContain("rounded-full"); expect(html).not.toContain("aria-label"); }); - it("shows no marker once a worktree has merged or conflicted", () => { + it("shows no enabled marker once a worktree has merged or conflicted", () => { for (const mergeStatus of ["merged", "conflict"] as const) { const html = markup( - buildItem({ worktreeBranch: "agent/feature-x", mergeStatus }) - .trailingElement + buildItem({ worktreeBranch: "agent/feature-x", mergeStatus }, ["s1"], { + showBranchTag: true, + }).trailingElement ); expect(html).not.toContain("aria-label"); } }); - it("keeps the git indicator on a working row, whose dot moves to the working slot", () => { + it("keeps the enabled git indicator on a working row", () => { const item = buildItem( { status: "running", worktreeBranch: "agent/feature-x" }, - [] + [], + { showBranchTag: true } ); expect(markup(item.trailingElement)).toContain( 'aria-label="Worktree branch: feature-x"' @@ -85,14 +126,17 @@ describe("buildSessionMenuItem trailing accessories", () => { }); }); -describe("buildSessionMenuItem PR state", () => { +describe("buildSessionMenuItem enabled PR state", () => { function prMarkup(status: string): string { return markup( buildItem({ branch: "feature-x" }, ["s1"], { - status, - number: 42, - url: "https://github.com/o/r/pull/42", - title: "Add thing", + showBranchTag: true, + pr: { + status, + number: 42, + url: "https://github.com/o/r/pull/42", + title: "Add thing", + }, }).trailingElement ); } @@ -115,10 +159,13 @@ describe("buildSessionMenuItem PR state", () => { it("still marks an in-flight worktree whose PR state is unrecognized", () => { const html = markup( buildItem({ worktreeBranch: "agent/feature-x" }, ["s1"], { - status: "pending_review", - number: 5, - url: "https://github.com/o/r/pull/5", - title: "Add thing", + showBranchTag: true, + pr: { + status: "pending_review", + number: 5, + url: "https://github.com/o/r/pull/5", + title: "Add thing", + }, }).trailingElement ); expect(html).toContain('aria-label="Worktree branch: feature-x"'); @@ -128,10 +175,13 @@ describe("buildSessionMenuItem PR state", () => { it("matches on the raw ref, so an agent worktree branch still labels short", () => { const html = markup( buildItem({ worktreeBranch: "agent/feature-x" }, ["s1"], { - status: "merged", - number: 7, - url: "https://github.com/o/r/pull/7", - title: "Add thing", + showBranchTag: true, + pr: { + status: "merged", + number: 7, + url: "https://github.com/o/r/pull/7", + title: "Add thing", + }, }).trailingElement ); expect(html).toContain('aria-label="Merged PR #7: feature-x"'); diff --git a/src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/index.tsx b/src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/index.tsx index 0c71092117..40bd6c978c 100644 --- a/src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/index.tsx +++ b/src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/index.tsx @@ -14,6 +14,7 @@ import { upsertSession, } from "@src/store/session"; import { agentLiveStatusAtom } from "@src/store/session/agentLiveStatusAtom"; +import { sessionBranchTagsVisibleAtom } from "@src/store/ui/sidebarAtom"; import { isImportedHistorySession } from "@src/util/session/sessionDispatch"; import { getSessionSearchText } from "@src/util/session/sessionSearch"; import { isPrimarySessionListSession } from "@src/util/session/sessionVisibility"; @@ -78,6 +79,7 @@ const SUBAGENT_SESSION_ID_SEGMENT = ":subagent:"; /** Max concurrent `es_get_child_sessions` calls when hydrating the sidebar. */ const SUBAGENT_QUERY_CONCURRENCY = 8; +const NO_SESSIONS: readonly Session[] = []; function parentSessionIdFor(session: Session): string | null { if (session.parentSessionId) return session.parentSessionId; @@ -177,6 +179,7 @@ export function useSessionMenuItems({ const { t: tCommon } = useTranslation(); const pagination = useAtomValue(sessionPaginationAtom); const agentLiveStatuses = useAtomValue(agentLiveStatusAtom); + const showBranchTags = useAtomValue(sessionBranchTagsVisibleAtom); // parentId → the parent's updated_at at query time. Children are re-fetched // only when the parent session changes, instead of re-querying every // visible session on every list refresh (that pattern issued 100+ @@ -402,9 +405,10 @@ export function useSessionMenuItems({ return map; }, [childSessionsByParent, visibleSessions]); - // Keyed off the listed rows, not `visibleSessions`: a repo only earns a PR - // fetch once one of its sessions is actually on screen. - const prForSession = useSessionPrStatuses(listedSessions); + // Do not mount any repo refresh work while branch tags are hidden. + const prForSession = useSessionPrStatuses( + showBranchTags ? listedSessions : NO_SESSIONS + ); const buildSessionRow = useCallback( (session: Session): NavigationMenuItem => @@ -415,9 +419,16 @@ export function useSessionMenuItems({ liveDetail: liveDetailForSession( agentLiveStatuses.get(session.session_id) ), - pr: prForSession(session), + showBranchTag: showBranchTags, + pr: showBranchTags ? prForSession(session) : undefined, }), - [agentLiveStatuses, prForSession, untitledSession, visitedSessions] + [ + agentLiveStatuses, + prForSession, + showBranchTags, + untitledSession, + visitedSessions, + ] ); const loadMoreRowFor = useCallback( diff --git a/src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/menuItemBuilders.tsx b/src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/menuItemBuilders.tsx index bb3919886d..9b7f9c827c 100644 --- a/src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/menuItemBuilders.tsx +++ b/src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/menuItemBuilders.tsx @@ -39,11 +39,9 @@ interface BuildSessionMenuItemParams { * question). Rendered as the row subtitle only while the session waits. */ liveDetail?: string; - /** - * PR the session's branch belongs to, from the sidebar's per-repo snapshot - * cache. Absent until the first fetch lands, or permanently for non-GitHub - * remotes — the row falls back to a plain branch glyph either way. - */ + /** Controls the optional branch/worktree and pull-request status tag. */ + showBranchTag?: boolean; + /** Pull request matched to this session's branch when tags are enabled. */ pr?: BranchPrSnapshot; } @@ -52,6 +50,7 @@ export function buildSessionMenuItem({ untitledSession, visitedSessions, liveDetail, + showBranchTag = false, pr, }: BuildSessionMenuItemParams): NavigationMenuItem { const inProgress = isSessionInProgress(session.status, session); @@ -60,11 +59,11 @@ export function buildSessionMenuItem({ session.updated_at || session.updated_time || session.created_at; const pendingAsking = isSessionPendingAsking(session); const statusDotTone = resolveSessionStatusDotTone(session, visitedSessions); - // A working row parks its dot in `workingIndicator` instead, so the trailing - // slot may hold the git marker alone. const statusDot = inProgress && !pendingAsking ? null : renderStatusDot(statusDotTone); - const gitIndicator = renderSessionGitIndicator(session, pr); + const gitIndicator = showBranchTag + ? renderSessionGitIndicator(session, pr) + : null; // The section header used to be the ONLY at-rest pin affordance, so pinning // was invisible wherever that header does not render (cloud scope strips // every separator) — and since the list is already recency-sorted, pinning a diff --git a/src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/useSessionPrStatuses.ts b/src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/useSessionPrStatuses.ts index f38a2b7910..f8223547f0 100644 --- a/src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/useSessionPrStatuses.ts +++ b/src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/useSessionPrStatuses.ts @@ -35,7 +35,7 @@ const logger = createLogger("useSessionPrStatuses"); * actually looks at. Older rows still render their branch glyph — they just * do not pull a repo into the fetch set on their own. */ -const MAX_SESSIONS_SCANNED = 60; +const MAX_SESSIONS_SCANNED = 30; export type SessionPrLookup = ( session: Session diff --git a/src/store/ui/__tests__/sidebarAtom.test.ts b/src/store/ui/__tests__/sidebarAtom.test.ts index f82d9d0a0f..8e5571eea6 100644 --- a/src/store/ui/__tests__/sidebarAtom.test.ts +++ b/src/store/ui/__tests__/sidebarAtom.test.ts @@ -1,11 +1,51 @@ import { createStore } from "jotai/vanilla"; +import { beforeEach } from "vitest"; import { + SESSION_BRANCH_TAGS_VISIBLE_STORAGE_KEY, clearSessionSidebarRevealAtom, requestSessionSidebarRevealAtom, + sessionBranchTagsVisibleAtom, sessionSidebarRevealRequestAtom, } from "../sidebarAtom"; +beforeEach(() => { + localStorage.removeItem(SESSION_BRANCH_TAGS_VISIBLE_STORAGE_KEY); +}); + +function hydratedStore(): ReturnType { + const store = createStore(); + store.sub(sessionBranchTagsVisibleAtom, () => undefined); + return store; +} + +describe("sessionBranchTagsVisibleAtom", () => { + it("hides branch tags by default", () => { + expect(hydratedStore().get(sessionBranchTagsVisibleAtom)).toBe(false); + }); + + it("persists an enabled choice for the future settings control", () => { + const writer = hydratedStore(); + writer.set(sessionBranchTagsVisibleAtom, true); + + expect( + JSON.parse( + localStorage.getItem(SESSION_BRANCH_TAGS_VISIBLE_STORAGE_KEY) ?? "null" + ) + ).toBe(true); + expect(hydratedStore().get(sessionBranchTagsVisibleAtom)).toBe(true); + }); + + it("falls back to hidden for a malformed stored value", () => { + localStorage.setItem( + SESSION_BRANCH_TAGS_VISIBLE_STORAGE_KEY, + JSON.stringify("visible") + ); + + expect(hydratedStore().get(sessionBranchTagsVisibleAtom)).toBe(false); + }); +}); + describe("requestSessionSidebarRevealAtom", () => { it("normalizes identities and increments repeated reveal requests", () => { const store = createStore(); diff --git a/src/store/ui/sidebarAtom.ts b/src/store/ui/sidebarAtom.ts index 9218fb364f..a061ed64cc 100644 --- a/src/store/ui/sidebarAtom.ts +++ b/src/store/ui/sidebarAtom.ts @@ -5,6 +5,7 @@ * Settings and session sidebars collapse and expand together. */ import { atom } from "jotai"; +import { atomWithStorage } from "jotai/utils"; // ============================================ // Constants @@ -14,6 +15,8 @@ export const DEFAULT_SIDEBAR_WIDTH = 240; export const MIN_SIDEBAR_WIDTH = 200; export const MAX_SIDEBAR_WIDTH = 320; export const COLLAPSED_SIDEBAR_WIDTH = 0; +export const SESSION_BRANCH_TAGS_VISIBLE_STORAGE_KEY = + "orgii:sidebar:sessionBranchTagsVisible"; // ============================================ // Shared collapsed persistence (localStorage) @@ -64,6 +67,24 @@ sidebarCollapsedAtom.debugLabel = "sidebarCollapsedAtom"; export const sidebarDraggingAtom = atom(false); sidebarDraggingAtom.debugLabel = "sidebarDraggingAtom"; +const storedSessionBranchTagsVisibleAtom = atomWithStorage( + SESSION_BRANCH_TAGS_VISIBLE_STORAGE_KEY, + false, + undefined, + { getOnInit: true } +); + +/** + * Whether session rows show branch/worktree and pull-request status tags. + * Defaults to hidden until a settings control exposes this preference. + */ +export const sessionBranchTagsVisibleAtom = atom( + (get) => get(storedSessionBranchTagsVisibleAtom) === true, + (_get, set, visible: boolean) => + set(storedSessionBranchTagsVisibleAtom, visible) +); +sessionBranchTagsVisibleAtom.debugLabel = "sessionBranchTagsVisibleAtom"; + export interface SessionSidebarRevealTarget { /** Independently replayable session row that should be selected and shown. */ sessionId: string;