diff --git a/docs/frontend-ui-audit-2026-07-30/NavigationSidebarStateCorrectness.md b/docs/frontend-ui-audit-2026-07-30/NavigationSidebarStateCorrectness.md new file mode 100644 index 000000000..d7e0b647b --- /dev/null +++ b/docs/frontend-ui-audit-2026-07-30/NavigationSidebarStateCorrectness.md @@ -0,0 +1,41 @@ +# Frontend UI Audit — Navigation Sidebar State Correctness + +**Files:** `NavigationSidebar.tsx` and the cloud-scoped session menu helpers +**Date:** 2026-07-30 +**Auditor:** ORGII coding session + +## D1 — Raw HTML vs Design System + +| Line | Element | Verdict | Reason | Suggested change | +|---|---|---|---|---| +| `NavigationSidebar.tsx:442-483` | Existing section header interaction | keep with reason | The change preserves the sidebar's existing keyboard-capable section header and shared icon button; it only corrects which persisted collapse state drives the presentation. | — | + +## D2 — Arbitrary Tailwind Value vs Token + +| Line | Value | Verdict | Reason | Suggested change | +|---|---|---|---|---| +| — | Existing sidebar classes | keep with reason | No Tailwind or color value changes are introduced. | — | + +## D3 — Hardcoded Sizes / Colors + +| Line | Value | Verdict | Reason | Suggested change | +|---|---|---|---|---| +| — | Existing section geometry | keep with reason | The change does not add or alter hardcoded dimensions or colors. | — | + +## D4 — Accessibility + +| Line | Element | Verdict | Reason | Suggested change | +|---|---|---|---|---| +| `NavigationSidebar.tsx:442-483` | Collapsible section header | keep with reason | Mouse, Enter, and Space activation remain aligned, and `aria-expanded` now consistently reflects the authoritative persisted state even while search filters visible rows. | — | + +## D5 — Visual Patterns Observed + +- Cloud-scoped local sessions retain the canonical sidebar row renderer and are sorted before pagination, so activity ordering cannot diverge between pages. +- Section collapse remains presentation state owned by the sidebar; search changes row visibility without silently rewriting or bypassing that preference. +- No parallel menu, button, empty state, or visual abstraction is introduced. + +## Summary + +- 0 fixes recommended +- 4 keep-with-reason findings +- 0 abstract candidates diff --git a/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/cloudScopedMenuItems.test.ts b/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/cloudScopedMenuItems.test.ts index 1a5ba07e5..eb8bba5aa 100644 --- a/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/cloudScopedMenuItems.test.ts +++ b/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/cloudScopedMenuItems.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import type { NavigationMenuItem } from "@src/scaffold/NavigationSidebar/components/NavigationMenu/config"; +import type { Session } from "@src/store/session"; import { CLOUD_MY_SESSIONS_LOAD_MORE_ID, @@ -9,6 +10,14 @@ import { } from "./cloudScopedMenuItems"; describe("buildCloudScopedMenuItems", () => { + const session = (sessionId: string, updatedAt: string): Session => ({ + session_id: sessionId, + status: "completed", + created_at: updatedAt, + updated_at: updatedAt, + }); + const sessionMap = (...sessions: Session[]) => + new Map(sessions.map((entry) => [entry.session_id, entry])); const localSections: NavigationMenuItem[] = [ { id: "separator-today", key: "separator-today", label: "Today" }, { id: "session-today", key: "session-today", label: "Today session" }, @@ -29,6 +38,7 @@ describe("buildCloudScopedMenuItems", () => { buildCloudScopedMenuItems({ cloudMenuItems: [], sessionMenuItems: localSections, + sessionById: sessionMap(), mySessionsLabel: "My sessions", }) ).toEqual(localSections); @@ -47,6 +57,10 @@ describe("buildCloudScopedMenuItems", () => { const result = buildCloudScopedMenuItems({ cloudMenuItems: teamItems, sessionMenuItems: localSections, + sessionById: sessionMap( + session("session-today", "2026-07-27T12:00:00Z"), + session("session-yesterday", "2026-07-26T12:00:00Z") + ), mySessionsLabel: "My sessions", }); @@ -62,6 +76,40 @@ describe("buildCloudScopedMenuItems", () => { ]); }); + it("restores one newest-activity queue after hidden subgroup ordering", () => { + const result = buildCloudScopedMenuItems({ + cloudMenuItems: [ + { + id: "separator-cloud-team-sessions", + key: "separator-cloud-team-sessions", + label: "Team sessions", + }, + ], + // Simulates pinned/workspace grouping putting an older row first. + sessionMenuItems: [ + { id: "older-pinned", key: "older-pinned", label: "Older pinned" }, + { id: "separator-workspace", key: "separator-workspace", label: "A" }, + { id: "newest", key: "newest", label: "Newest" }, + { id: "middle", key: "middle", label: "Middle" }, + ], + sessionById: sessionMap( + session("older-pinned", "2026-07-25T12:00:00Z"), + session("newest", "2026-07-27T12:00:00Z"), + session("middle", "2026-07-26T12:00:00Z") + ), + mySessionsLabel: "My sessions", + }); + + const mySectionIndex = result.findIndex( + (item) => item.id === `separator-${CLOUD_MY_SESSIONS_SECTION_ID}` + ); + expect(result.slice(mySectionIndex + 1).map((item) => item.id)).toEqual([ + "newest", + "middle", + "older-pinned", + ]); + }); + it("renders the My sessions section even when it has no local rows", () => { expect( buildCloudScopedMenuItems({ @@ -73,6 +121,7 @@ describe("buildCloudScopedMenuItems", () => { }, ], sessionMenuItems: [], + sessionById: sessionMap(), mySessionsLabel: "My sessions", }) ).toEqual([ @@ -123,6 +172,14 @@ describe("buildCloudScopedMenuItems", () => { }, ], sessionMenuItems: sessionItems, + sessionById: sessionMap( + ...Array.from({ length: 24 }, (_, index) => + session( + `session-${index}`, + new Date(Date.UTC(2026, 6, 27, 0, 0, 24 - index)).toISOString() + ) + ) + ), mySessionsLabel: "My sessions", loadMoreLabel: "Load more", }); @@ -159,6 +216,14 @@ describe("buildCloudScopedMenuItems", () => { }, ], sessionMenuItems: sessionItems, + sessionById: sessionMap( + ...Array.from({ length: 21 }, (_, index) => + session( + `session-${index}`, + new Date(Date.UTC(2026, 6, 27, 0, 0, 21 - index)).toISOString() + ) + ) + ), mySessionsLabel: "My sessions", mySessionsVisibleCount: 20, }); diff --git a/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/cloudScopedMenuItems.ts b/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/cloudScopedMenuItems.ts index e1d420b94..c4a99eddc 100644 --- a/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/cloudScopedMenuItems.ts +++ b/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/cloudScopedMenuItems.ts @@ -2,8 +2,10 @@ import { MoreHorizontal } from "lucide-react"; import type { ReactNode } from "react"; import type { NavigationMenuItem } from "@src/scaffold/NavigationSidebar/components/NavigationMenu/config"; +import type { Session } from "@src/store/session"; import { separator } from "../useSessionMenuItems/menuItemBuilders"; +import { sortSessionsByActivity } from "../workstationSidebarData"; export const CLOUD_MY_SESSIONS_SECTION_ID = "cloud-my-sessions"; export const CLOUD_TEAM_SESSIONS_SECTION_ID = "cloud-team-sessions"; @@ -14,6 +16,7 @@ export const CLOUD_MY_SESSIONS_LOAD_MORE_ID = "cloud-my-sessions-next-page"; interface BuildCloudScopedMenuItemsParams { cloudMenuItems: readonly NavigationMenuItem[]; sessionMenuItems: readonly NavigationMenuItem[]; + sessionById: ReadonlyMap; mySessionsLabel: string; mySessionsVisibleCount?: number; loadMoreLabel?: string; @@ -52,14 +55,40 @@ export function buildCloudSectionLoadMoreItem({ }; } +export function orderSessionMenuRowsByActivity( + rows: readonly NavigationMenuItem[], + sessionById: ReadonlyMap +): NavigationMenuItem[] { + const rowBySessionId = new Map(); + const unmatchedRows: NavigationMenuItem[] = []; + + for (const row of rows) { + if (sessionById.has(row.id)) rowBySessionId.set(row.id, row); + else unmatchedRows.push(row); + } + + const matchedSessions = Array.from(rowBySessionId.keys()) + .map((sessionId) => sessionById.get(sessionId)) + .filter((session): session is Session => session !== undefined); + + return [ + ...sortSessionsByActivity(matchedSessions).flatMap((session) => { + const row = rowBySessionId.get(session.session_id); + return row ? [row] : []; + }), + ...unmatchedRows, + ]; +} + /** * Cloud scope has two top-level sections: shared team sessions and the - * viewer's own sessions. Local grouping separators are removed so every - * regular local row belongs to the single "My sessions" section. + * viewer's own sessions. Local grouping separators are removed and local rows + * form one canonical newest-activity-first queue before pagination. */ export function buildCloudScopedMenuItems({ cloudMenuItems, sessionMenuItems, + sessionById, mySessionsLabel, mySessionsVisibleCount = CLOUD_SESSION_SECTION_PAGE_SIZE, loadMoreLabel = "Load more", @@ -69,7 +98,10 @@ export function buildCloudScopedMenuItems({ const backendPaginationItems = sessionMenuItems.filter( isSessionPaginationMenuItem ); - const localRows = sessionMenuItems.filter(isCloudScopedLocalRow); + const localRows = orderSessionMenuRowsByActivity( + sessionMenuItems.filter(isCloudScopedLocalRow), + sessionById + ); const visibleLocalRows = localRows.slice(0, mySessionsVisibleCount); const hasHiddenLoadedRows = localRows.length > visibleLocalRows.length; const readyBackendPaginationItem = backendPaginationItems.find( diff --git a/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/sidebarConnector.menuDecoration.ts b/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/sidebarConnector.menuDecoration.ts index 1d73c862c..6f46aa314 100644 --- a/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/sidebarConnector.menuDecoration.ts +++ b/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/sidebarConnector.menuDecoration.ts @@ -185,6 +185,7 @@ export function useWorkstationSidebarMenuDecoration({ // Cloud rows already carry Replay/Fork actions, so only local rows // use the regular session action decoration. sessionMenuItems: decorateSessionRowActions(sessionSidebarMenuItems), + sessionById: sessionMap, mySessionsLabel: t("cloud.sidebar.mySessions"), mySessionsVisibleCount: cloudMySessionsVisibleCount, loadMoreLabel: tCommon("common:actions.loadMore", "Load more"), @@ -193,6 +194,7 @@ export function useWorkstationSidebarMenuDecoration({ cloudMenuItems, cloudMySessionsVisibleCount, decorateSessionRowActions, + sessionMap, sessionSidebarMenuItems, t, tCommon, diff --git a/src/scaffold/NavigationSidebar/variants/NavigationSidebar.tsx b/src/scaffold/NavigationSidebar/variants/NavigationSidebar.tsx index f52d745ab..505a0b219 100644 --- a/src/scaffold/NavigationSidebar/variants/NavigationSidebar.tsx +++ b/src/scaffold/NavigationSidebar/variants/NavigationSidebar.tsx @@ -27,6 +27,7 @@ import type { NavigationMenuRowAction, } from "../components/NavigationMenu/config"; import type { SidebarTab } from "../types"; +import { isNavigationSectionCollapsed } from "./navigationSectionCollapse"; // ============================================ // Types @@ -438,10 +439,11 @@ const NavigationSidebar: React.FC = React.memo( /> ) : ( sections.map((section) => { - const isSectionCollapsed = - !hasSearchInput && - collapsibleSections && - collapsedSections.has(section.id); + const isSectionCollapsed = isNavigationSectionCollapsed({ + collapsibleSections, + collapsedSectionIds: collapsedSections, + sectionId: section.id, + }); return (
@@ -453,9 +455,7 @@ const NavigationSidebar: React.FC = React.memo( tabIndex={0} aria-expanded={!isSectionCollapsed} className={`${isSectionCollapsed ? "" : "mb-px"} group/section-title flex h-7 cursor-pointer items-center gap-2 pl-2`} - onClick={() => { - if (!hasSearchInput) toggleSection(section.id); - }} + onClick={() => toggleSection(section.id)} onKeyDown={(event) => { if ( event.target !== event.currentTarget || @@ -464,7 +464,7 @@ const NavigationSidebar: React.FC = React.memo( return; } event.preventDefault(); - if (!hasSearchInput) toggleSection(section.id); + toggleSection(section.id); }} > @@ -476,9 +476,7 @@ const NavigationSidebar: React.FC = React.memo( isSectionCollapsed ? ChevronRight : ChevronDown } label={section.title ?? section.id} - onClick={() => { - if (!hasSearchInput) toggleSection(section.id); - }} + onClick={() => toggleSection(section.id)} /> {section.headerActions && ( diff --git a/src/scaffold/NavigationSidebar/variants/navigationSectionCollapse.test.ts b/src/scaffold/NavigationSidebar/variants/navigationSectionCollapse.test.ts new file mode 100644 index 000000000..2989e17a6 --- /dev/null +++ b/src/scaffold/NavigationSidebar/variants/navigationSectionCollapse.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, it } from "vitest"; + +import { isNavigationSectionCollapsed } from "./navigationSectionCollapse"; + +describe("isNavigationSectionCollapsed", () => { + it("preserves a collapsed section while its rows are filtered", () => { + expect( + isNavigationSectionCollapsed({ + collapsibleSections: true, + collapsedSectionIds: new Set(["cloud-my-sessions"]), + sectionId: "cloud-my-sessions", + }) + ).toBe(true); + }); + + it("does not collapse sections when section collapsing is disabled", () => { + expect( + isNavigationSectionCollapsed({ + collapsibleSections: false, + collapsedSectionIds: new Set(["cloud-my-sessions"]), + sectionId: "cloud-my-sessions", + }) + ).toBe(false); + }); +}); diff --git a/src/scaffold/NavigationSidebar/variants/navigationSectionCollapse.ts b/src/scaffold/NavigationSidebar/variants/navigationSectionCollapse.ts new file mode 100644 index 000000000..00f4e8dad --- /dev/null +++ b/src/scaffold/NavigationSidebar/variants/navigationSectionCollapse.ts @@ -0,0 +1,17 @@ +export interface NavigationSectionCollapseState { + collapsibleSections: boolean; + collapsedSectionIds: ReadonlySet; + sectionId: string; +} + +/** + * Section collapse state is presentation state and must remain authoritative + * while the visible rows are filtered by search. + */ +export function isNavigationSectionCollapsed({ + collapsibleSections, + collapsedSectionIds, + sectionId, +}: NavigationSectionCollapseState): boolean { + return collapsibleSections && collapsedSectionIds.has(sectionId); +}