From 6eb2249620085b757bddebb3d95471d087c5967d Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 17 Aug 2026 01:28:42 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=B1=20perf:=20Instant=20Mobile=20View?= =?UTF-8?q?=20Switching=20+=20Uniform=20Sidebar=20Toggle=20(#14913)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * โšก perf: Stabilize the Assistants Map Context Value * โšก perf: Start the Mobile Drawer Slide Before the State-Flip Commit * ๐Ÿ“ฑ style: Mirror the Header Sidebar Toggle in the Mobile Drawer * ๐Ÿงท fix: Apply Reduced-Motion Flips Synchronously, Drop Stale Deferred Flips * ๐ŸŽจ refactor: Promote the Sidebar Toggle Look to a Button Variant * ๐Ÿงท fix: Drive Focus and the Release Deadline From the Commit, Not Timers * ๐Ÿšช fix: Route Every Sidebar Mutation Through the Animated Toggle * ๐Ÿ› fix: Carry Navigation and Focus Past the Slide, Toggle the Latest Intent * ๐Ÿ†• fix: Slide Before the New-Chat Reset, Drop Superseded Deferred Flips --- .../src/components/Chat/Menus/OpenSidebar.tsx | 30 +-- client/src/components/Conversations/Convo.tsx | 15 +- .../UnifiedSidebar/ConversationsSection.tsx | 28 +- .../UnifiedSidebar/UnifiedSidebar.tsx | 23 +- .../UnifiedSidebar/mobile/BottomBar.tsx | 40 ++- .../UnifiedSidebar/mobile/Header.tsx | 39 ++- .../mobile/__tests__/BottomBar.spec.tsx | 101 +++++++ .../mobile/__tests__/Header.spec.tsx | 50 +++- .../Agents/__tests__/useAgentsMap.spec.tsx | 62 +++++ .../__tests__/useAssistantsMap.spec.tsx | 82 ++++++ .../src/hooks/Assistants/useAssistantsMap.ts | 25 +- .../Nav/__tests__/useDrawerSwipe.spec.tsx | 251 +++++++++++++++++- .../Nav/__tests__/useSidebarToggle.spec.tsx | 177 ++++++++++++ client/src/hooks/Nav/useDrawerSwipe.ts | 183 ++++++++++++- client/src/hooks/Nav/useSidebarToggle.ts | 76 ++++++ client/src/hooks/useKeyboardShortcuts.ts | 31 ++- client/src/routes/Root.tsx | 11 +- .../client/src/components/Button.spec.tsx | 12 + packages/client/src/components/Button.tsx | 11 + 19 files changed, 1146 insertions(+), 101 deletions(-) create mode 100644 client/src/components/UnifiedSidebar/mobile/__tests__/BottomBar.spec.tsx create mode 100644 client/src/hooks/Agents/__tests__/useAgentsMap.spec.tsx create mode 100644 client/src/hooks/Assistants/__tests__/useAssistantsMap.spec.tsx create mode 100644 client/src/hooks/Nav/__tests__/useSidebarToggle.spec.tsx create mode 100644 client/src/hooks/Nav/useSidebarToggle.ts diff --git a/client/src/components/Chat/Menus/OpenSidebar.tsx b/client/src/components/Chat/Menus/OpenSidebar.tsx index 8aa47b2b2f..dfa311d551 100644 --- a/client/src/components/Chat/Menus/OpenSidebar.tsx +++ b/client/src/components/Chat/Menus/OpenSidebar.tsx @@ -1,10 +1,7 @@ -import { startTransition } from 'react'; -import { useSetRecoilState } from 'recoil'; import { TooltipAnchor, Button, Sidebar } from '@librechat/client'; import { useShortcutAriaKey, useShortcutHint } from '~/hooks/useKeyboardShortcuts'; +import useSidebarToggle from '~/hooks/Nav/useSidebarToggle'; import { useLocalize } from '~/hooks'; -import { cn } from '~/utils'; -import store from '~/store'; export const CLOSE_SIDEBAR_ID = 'close-sidebar-button'; export const OPEN_SIDEBAR_ID = 'open-sidebar-button'; @@ -22,17 +19,21 @@ export default function OpenSidebar({ testId?: string; }) { const localize = useLocalize(); - const setSidebarExpanded = useSetRecoilState(store.sidebarExpanded); + const { setSidebarOpen } = useSidebarToggle(); const tooltipDescription = useShortcutHint('toggleSidebar', localize('com_nav_open_sidebar')); const ariaKey = useShortcutAriaKey('toggleSidebar'); const handleClick = () => { - startTransition(() => { - setSidebarExpanded(true); - }); - setTimeout(() => { - document.getElementById(CLOSE_SIDEBAR_ID)?.focus(); - }, 250); + const mode = setSidebarOpen(true); + if (mode === 'none') { + /** Desktop only: the expanded panel claims `CLOSE_SIDEBAR_ID` and has + * no commit-driven handoff of its own. The mobile drawer focuses its + * toggle from the commit itself โ€” a second timer there would steal + * focus back from a keyboard user who has already tabbed onward. */ + setTimeout(() => { + document.getElementById(CLOSE_SIDEBAR_ID)?.focus(); + }, 250); + } }; return ( @@ -42,16 +43,13 @@ export default function OpenSidebar({ + + }> + + ); } diff --git a/client/src/components/UnifiedSidebar/mobile/__tests__/BottomBar.spec.tsx b/client/src/components/UnifiedSidebar/mobile/__tests__/BottomBar.spec.tsx new file mode 100644 index 0000000000..7db76f18e3 --- /dev/null +++ b/client/src/components/UnifiedSidebar/mobile/__tests__/BottomBar.spec.tsx @@ -0,0 +1,101 @@ +import React from 'react'; +import { RecoilRoot } from 'recoil'; +import { render, screen, fireEvent } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import type { NavLink } from '~/common'; + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => key, +})); + +jest.mock('~/hooks/useKeyboardShortcuts', () => ({ + useShortcutAriaKey: () => 'Meta+Shift+O', +})); + +jest.mock('@librechat/client', () => ({ + Button: ({ + children, + asChild: _asChild, + ...props + }: { + children: React.ReactNode; + asChild?: boolean; + }) =>
{children}
, +})); + +jest.mock('~/Providers', () => ({ + useActivePanel: () => ({ active: 'conversations', setActive: jest.fn() }), + resolveActivePanel: () => 'conversations', + DEFAULT_PANEL: 'conversations', +})); + +jest.mock('~/components/Nav/SearchBar', () => ({ + __esModule: true, + default: () =>
, +})); + +/** The heavy reset itself is the unit under order-test; only the navigation + * internals below useNewChat are stubbed. */ +const mockNewConversation = jest.fn(); +jest.mock('~/hooks/useNewConvo', () => ({ + __esModule: true, + default: () => ({ newConversation: mockNewConversation }), +})); + +import BottomBar from '../BottomBar'; + +const links = [] as NavLink[]; + +describe('mobile bottom bar โ€” new chat ordering', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + /** + * The drawer close must start BEFORE the conversation reset: run + * synchronously, the reset's cache clearing and navigation flush in the + * tap's task and stall the slide's first frame. The reset rides the + * close's `afterSlide` callback instead. + */ + it('closes the drawer first and defers the conversation reset to afterSlide', () => { + const order: string[] = []; + let afterSlide: (() => void) | undefined; + const onNewChat = jest.fn((callback?: () => void) => { + order.push('close'); + afterSlide = callback; + }); + mockNewConversation.mockImplementation(() => { + order.push('reset'); + }); + + render( + + + + + , + ); + fireEvent.click(screen.getByTestId('nav-new-chat-fab')); + + expect(order).toEqual(['close']); + expect(afterSlide).toBeDefined(); + + afterSlide?.(); + expect(order).toEqual(['close', 'reset']); + }); + + it('leaves modified clicks to the browser (new tab)', () => { + const onNewChat = jest.fn(); + render( + + + + + , + ); + fireEvent.click(screen.getByTestId('nav-new-chat-fab'), { ctrlKey: true }); + + expect(onNewChat).not.toHaveBeenCalled(); + expect(mockNewConversation).not.toHaveBeenCalled(); + }); +}); diff --git a/client/src/components/UnifiedSidebar/mobile/__tests__/Header.spec.tsx b/client/src/components/UnifiedSidebar/mobile/__tests__/Header.spec.tsx index 4e723dac21..9c63b5be0e 100644 --- a/client/src/components/UnifiedSidebar/mobile/__tests__/Header.spec.tsx +++ b/client/src/components/UnifiedSidebar/mobile/__tests__/Header.spec.tsx @@ -1,3 +1,4 @@ +import React from 'react'; import { render, screen } from '@testing-library/react'; import type { NavLink } from '~/common'; @@ -10,9 +11,17 @@ jest.mock('~/hooks/useKeyboardShortcuts', () => ({ })); jest.mock('@librechat/client', () => ({ - Button: ({ children, ...props }: React.ComponentProps<'button'>) => ( - - ), + Button: jest + .requireActual('react') + .forwardRef< + HTMLButtonElement, + React.ComponentProps<'button'> + >(({ children, ...props }, ref) => ( + + )), + Sidebar: (props: React.ComponentProps<'svg'>) => , Skeleton: () =>
, })); @@ -70,4 +79,39 @@ describe('mobile drawer header', () => { expect(screen.getByLabelText('com_nav_close_sidebar')).toHaveAttribute('tabindex', '-1'); }); + + /** + * Opening makes the chat pane inert, so keyboard/AT focus must move into + * the drawer. The commit itself drives the handoff โ€” a wall-clock timer + * races the deferred state flip and silently misses when the flip + * outlasts it (the id does not exist until `expanded` commits). + */ + it('moves focus to the toggle when the drawer opens', () => { + const { rerender } = render(
); + expect(document.activeElement).toBe(document.body); + + rerender(
); + + expect(document.activeElement).toBe(screen.getByTestId('close-sidebar-button')); + }); + + it('never steals focus while closed', () => { + render(
); + + expect(document.activeElement).toBe(document.body); + }); + + /** + * The toggle mirrors the chat header's OpenSidebar โ€” same icon, same shared + * Button variant, same far-left slot โ€” so the drawer reads as the one + * persistent control flipping state rather than a new X appearing elsewhere. + */ + it('leads the row with the shared header-action toggle', () => { + const { container } = render(
); + + const toggle = screen.getByTestId('close-sidebar-button'); + expect(container.firstElementChild?.firstElementChild).toBe(toggle); + expect(toggle).toHaveAttribute('variant', 'header-action'); + expect(toggle.querySelector('[data-testid="sidebar-icon"]')).not.toBeNull(); + }); }); diff --git a/client/src/hooks/Agents/__tests__/useAgentsMap.spec.tsx b/client/src/hooks/Agents/__tests__/useAgentsMap.spec.tsx new file mode 100644 index 0000000000..8632d1b3e0 --- /dev/null +++ b/client/src/hooks/Agents/__tests__/useAgentsMap.spec.tsx @@ -0,0 +1,62 @@ +import React from 'react'; +import { renderHook, waitFor } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { EModelEndpoint, QueryKeys, dataService } from 'librechat-data-provider'; +import useAgentsMap from '../useAgentsMap'; + +/** `dataService` methods are non-configurable, so the HTTP boundary is mocked + * at the module seam; everything else stays the real library. */ +jest.mock('librechat-data-provider', () => { + const actual = + jest.requireActual('librechat-data-provider'); + return { + ...actual, + dataService: { + ...actual.dataService, + listAgents: jest.fn(), + }, + }; +}); + +/** + * Root feeds this map straight into `AgentsMapContext.Provider`, so its + * referential stability decides whether every consumer โ€” message rows, + * conversation icons, subagent cards โ€” re-renders on unrelated Root renders + * (e.g. the mobile drawer toggle). Real react-query; only HTTP is stubbed. + */ +describe('useAgentsMap', () => { + const setup = () => { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + client.setQueryData([QueryKeys.endpoints], { + [EModelEndpoint.agents]: {}, + }); + (dataService.listAgents as jest.Mock).mockResolvedValue({ + object: 'list', + data: [{ id: 'agent_1', name: 'Agent One' }], + first_id: 'agent_1', + last_id: 'agent_1', + has_more: false, + after: null, + }); + const wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} + ); + return renderHook(() => useAgentsMap({ isAuthenticated: true }), { wrapper }); + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('keeps the agents map referentially stable across unrelated re-renders', async () => { + const { result, rerender } = setup(); + await waitFor(() => expect(result.current?.agent_1).toBeDefined()); + + const first = result.current; + rerender(); + + expect(result.current).toBe(first); + }); +}); diff --git a/client/src/hooks/Assistants/__tests__/useAssistantsMap.spec.tsx b/client/src/hooks/Assistants/__tests__/useAssistantsMap.spec.tsx new file mode 100644 index 0000000000..c9d108198a --- /dev/null +++ b/client/src/hooks/Assistants/__tests__/useAssistantsMap.spec.tsx @@ -0,0 +1,82 @@ +import React from 'react'; +import { renderHook, waitFor } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { EModelEndpoint, QueryKeys, dataService } from 'librechat-data-provider'; +import useAssistantsMap from '../useAssistantsMap'; + +/** `dataService` methods are non-configurable, so the HTTP boundary is mocked + * at the module seam; everything else stays the real library. */ +jest.mock('librechat-data-provider', () => { + const actual = + jest.requireActual('librechat-data-provider'); + return { + ...actual, + dataService: { + ...actual.dataService, + listAssistants: jest.fn(), + }, + }; +}); + +/** + * Root feeds this map straight into `AssistantsMapContext.Provider`, so its + * referential stability decides whether every consumer โ€” message rows + * included โ€” re-renders on unrelated Root renders (e.g. the mobile drawer + * toggle). Real react-query against a seeded cache; only HTTP is stubbed. + */ +describe('useAssistantsMap', () => { + const listResponse = { + object: 'list', + data: [{ id: 'asst_1', name: 'Assistant One' }], + first_id: 'asst_1', + last_id: 'asst_1', + has_more: false, + }; + + const setup = (isAuthenticated: boolean) => { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + client.setQueryData([QueryKeys.endpoints], { + [EModelEndpoint.assistants]: { userProvide: false }, + }); + (dataService.listAssistants as jest.Mock).mockResolvedValue(listResponse); + const wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} + ); + return renderHook(() => useAssistantsMap({ isAuthenticated }), { wrapper }); + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('keeps the composite map referentially stable across unrelated re-renders', async () => { + const { result, rerender } = setup(true); + await waitFor(() => expect(result.current?.[EModelEndpoint.assistants]?.asst_1).toBeDefined()); + + const first = result.current; + rerender(); + + expect(result.current).toBe(first); + }); + + it('keeps each endpoint map stable across re-renders (memoized select)', async () => { + const { result, rerender } = setup(true); + await waitFor(() => expect(result.current?.[EModelEndpoint.assistants]?.asst_1).toBeDefined()); + + const firstInner = result.current[EModelEndpoint.assistants]; + rerender(); + + expect(result.current[EModelEndpoint.assistants]).toBe(firstInner); + }); + + it('is stable while unauthenticated (empty maps do not churn identity)', () => { + const { result, rerender } = setup(false); + + const first = result.current; + rerender(); + + expect(result.current).toBe(first); + }); +}); diff --git a/client/src/hooks/Assistants/useAssistantsMap.ts b/client/src/hooks/Assistants/useAssistantsMap.ts index eb44bef8d0..9894498182 100644 --- a/client/src/hooks/Assistants/useAssistantsMap.ts +++ b/client/src/hooks/Assistants/useAssistantsMap.ts @@ -1,18 +1,26 @@ +import { useMemo } from 'react'; import { EModelEndpoint } from 'librechat-data-provider'; import type { TAssistantsMap } from 'librechat-data-provider'; import { useListAssistantsQuery } from '~/data-provider'; import { mapAssistants } from '~/utils'; +/** + * Root feeds this straight into `AssistantsMapContext.Provider`, so the + * returned reference must only change when the underlying data does. The + * previous bare object literal (and `= {}` defaults) minted a new value every + * render, which re-rendered every context consumer โ€” message rows included โ€” + * on any unrelated Root render, e.g. the mobile drawer toggle. + */ export default function useAssistantsMap({ isAuthenticated, }: { isAuthenticated: boolean; -}): TAssistantsMap | undefined { - const { data: assistants = {} } = useListAssistantsQuery(EModelEndpoint.assistants, undefined, { +}): TAssistantsMap { + const { data: assistants } = useListAssistantsQuery(EModelEndpoint.assistants, undefined, { select: (res) => mapAssistants(res.data), enabled: isAuthenticated, }); - const { data: azureAssistants = {} } = useListAssistantsQuery( + const { data: azureAssistants } = useListAssistantsQuery( EModelEndpoint.azureAssistants, undefined, { @@ -21,8 +29,11 @@ export default function useAssistantsMap({ }, ); - return { - [EModelEndpoint.assistants]: assistants, - [EModelEndpoint.azureAssistants]: azureAssistants, - }; + return useMemo( + () => ({ + [EModelEndpoint.assistants]: assistants ?? {}, + [EModelEndpoint.azureAssistants]: azureAssistants ?? {}, + }), + [assistants, azureAssistants], + ); } diff --git a/client/src/hooks/Nav/__tests__/useDrawerSwipe.spec.tsx b/client/src/hooks/Nav/__tests__/useDrawerSwipe.spec.tsx index d60112864f..895e3d0203 100644 --- a/client/src/hooks/Nav/__tests__/useDrawerSwipe.spec.tsx +++ b/client/src/hooks/Nav/__tests__/useDrawerSwipe.spec.tsx @@ -1,5 +1,9 @@ import { renderHook } from '@testing-library/react'; -import useDrawerSwipe, { findHorizontalScrollBlocker } from '../useDrawerSwipe'; +import useDrawerSwipe, { + findHorizontalScrollBlocker, + getPendingDrawerFlip, + kickDrawerAnimation, +} from '../useDrawerSwipe'; import { MOBILE_DRAWER_ID } from '~/components/UnifiedSidebar/constants'; const DRAWER_WIDTH = 375; @@ -432,6 +436,251 @@ describe('useDrawerSwipe โ€” interruptions and re-arming', () => { }); }); +describe('useDrawerSwipe โ€” kickDrawerAnimation (button toggles)', () => { + it('slides both elements before the state flip runs, then hands off to classes', () => { + jest.useFakeTimers(); + const harness = setup(false); + /** The sync rAF mock collapses the two-frame deferral, so capturing the + * transform inside `applyState` proves the slide started first. */ + const transformAtApply: string[] = []; + kickDrawerAnimation(true, () => { + transformAtApply.push(harness.drawer.style.transform); + }); + + expect(transformAtApply).toEqual(['translate3d(0, 0, 0)']); + expect(harness.pane.style.transform).toBe('translateX(100%)'); + expect(harness.drawer.style.willChange).toBe('transform'); + expect(harness.onOpenChange).not.toHaveBeenCalled(); + + harness.rerender({ open: true, enabled: true }); + expect(harness.drawer.style.transform).toBe('translate3d(0, 0, 0)'); + + jest.runAllTimers(); + expect(harness.drawer.style.transform).toBe(''); + expect(harness.drawer.style.willChange).toBe(''); + expect(harness.pane.style.transform).toBe('translateX(100%)'); + jest.useRealTimers(); + harness.unmount(); + }); + + it('applies state immediately when the hook is disabled (desktop width animation)', () => { + const harness = setup(false); + harness.rerender({ open: false, enabled: false }); + + const applyState = jest.fn(); + kickDrawerAnimation(true, applyState); + + expect(applyState).toHaveBeenCalledTimes(1); + expect(harness.drawer.style.transform).toBe(''); + harness.unmount(); + }); + + it('applies state immediately after unmount', () => { + const harness = setup(false); + harness.unmount(); + + const applyState = jest.fn(); + kickDrawerAnimation(true, applyState); + + expect(applyState).toHaveBeenCalledTimes(1); + }); + + it('retargets an in-flight slide when the user toggles back', () => { + jest.useFakeTimers(); + const harness = setup(false); + + kickDrawerAnimation(true, jest.fn()); + harness.rerender({ open: true, enabled: true }); + kickDrawerAnimation(false, jest.fn()); + + expect(harness.drawer.style.transform).toBe('translate3d(-100%, 0, 0)'); + expect(harness.pane.style.transform).toBe('translate3d(0, 0, 0)'); + + harness.rerender({ open: false, enabled: true }); + jest.runAllTimers(); + expect(harness.drawer.style.transform).toBe(''); + expect(harness.pane.style.transform).toBe(''); + jest.useRealTimers(); + harness.unmount(); + }); + + it('does not re-start a slide already heading to the same target', () => { + jest.useFakeTimers(); + const harness = setup(false); + + kickDrawerAnimation(true, jest.fn()); + harness.drawer.style.transform = 'translate3d(-10px, 0, 0)'; + kickDrawerAnimation(true, jest.fn()); + + expect(harness.drawer.style.transform).toBe('translate3d(-10px, 0, 0)'); + jest.useRealTimers(); + harness.unmount(); + }); + + it('defers to a drag that owns the inline styles but still applies state', () => { + const harness = setup(false); + harness.swipe( + harness.pane, + [ + { x: 20, y: 100, t: 0 }, + { x: 120, y: 104, t: 50 }, + ], + false, + ); + + const applyState = jest.fn(); + kickDrawerAnimation(true, applyState); + + expect(harness.pane.style.transform).toBe('translate3d(100px, 0, 0)'); + expect(applyState).toHaveBeenCalledTimes(1); + harness.pane.dispatchEvent(touchEvent('touchend', [], 400)); + harness.unmount(); + }); + + it('snaps without a transition under reduced motion', () => { + jest.useFakeTimers(); + const harness = setup(false, true); + + kickDrawerAnimation(true, jest.fn()); + + expect(harness.drawer.style.transition).toBe('none'); + expect(harness.drawer.style.transform).toBe(''); + + harness.rerender({ open: true, enabled: true }); + jest.runAllTimers(); + expect(harness.drawer.style.transition).not.toBe('none'); + jest.useRealTimers(); + harness.unmount(); + }); + + /** Deferring past the 80ms transition-restore window would let the state + * flip animate at full duration on a slow-frame device โ€” for exactly the + * users who opted out of motion. */ + it('applies state synchronously under reduced motion, without frame deferral', () => { + const harness = setup(false, true); + (window.requestAnimationFrame as jest.Mock).mockImplementation(() => 0); + + const applyState = jest.fn(); + kickDrawerAnimation(true, applyState); + + expect(applyState).toHaveBeenCalledTimes(1); + harness.unmount(); + }); + + it('drops a deferred state flip when the animator is torn down mid-deferral', () => { + const harness = setup(false); + const queued: FrameRequestCallback[] = []; + (window.requestAnimationFrame as jest.Mock).mockImplementation((callback) => { + queued.push(callback); + return queued.length; + }); + + const applyState = jest.fn(); + kickDrawerAnimation(true, applyState); + harness.rerender({ open: false, enabled: false }); + while (queued.length) { + queued.shift()?.(0); + } + + expect(applyState).not.toHaveBeenCalled(); + expect(harness.drawer.style.transform).toBe(''); + harness.unmount(); + }); + + /** The release deadline must run on the same clock as the staged frame: + * armed at kick time, a delayed first frame lets the wall-clock timeout + * clear the settle first, and the frame then aborts the slide. */ + it('waits for the staged frame before starting the release deadline', () => { + jest.useFakeTimers(); + const harness = setup(false); + const queued: FrameRequestCallback[] = []; + (window.requestAnimationFrame as jest.Mock).mockImplementation((callback) => { + queued.push(callback); + return queued.length; + }); + + kickDrawerAnimation(true, jest.fn()); + jest.advanceTimersByTime(500); + queued.shift()?.(0); + + expect(harness.drawer.style.transform).toBe('translate3d(0, 0, 0)'); + expect(harness.pane.style.transform).toBe('translateX(100%)'); + + harness.rerender({ open: true, enabled: true }); + jest.advanceTimersByTime(400); + expect(harness.drawer.style.transform).toBe(''); + expect(harness.pane.style.transform).toBe('translateX(100%)'); + jest.useRealTimers(); + harness.unmount(); + }); + + /** A superseded flip must never transiently commit the old direction โ€” + * its commit would re-register the animator and get the NEWER flip + * dropped, stranding the drawer at the first intent. */ + it('discards a superseded deferred flip so only the latest intent commits', () => { + jest.useFakeTimers(); + const harness = setup(false); + const queued: FrameRequestCallback[] = []; + (window.requestAnimationFrame as jest.Mock).mockImplementation((callback) => { + queued.push(callback); + return queued.length; + }); + + const applyOpen = jest.fn(); + const applyClose = jest.fn(); + kickDrawerAnimation(true, applyOpen); + kickDrawerAnimation(false, applyClose); + while (queued.length) { + queued.shift()?.(0); + } + + expect(applyOpen).not.toHaveBeenCalled(); + expect(applyClose).toHaveBeenCalledTimes(1); + jest.runAllTimers(); + jest.useRealTimers(); + harness.unmount(); + }); + + /** A desktop toggle right after a breakpoint cross must invert the + * committed value, not a leftover mobile intent. */ + it('clears the pending flip target when the animator tears down', () => { + const harness = setup(false); + const queued: FrameRequestCallback[] = []; + (window.requestAnimationFrame as jest.Mock).mockImplementation((callback) => { + queued.push(callback); + return queued.length; + }); + + kickDrawerAnimation(true, jest.fn()); + expect(getPendingDrawerFlip()).toBe(true); + harness.rerender({ open: false, enabled: false }); + + expect(getPendingDrawerFlip()).toBeNull(); + harness.unmount(); + }); + + it('applies the deferred state flip when the animator is still live', () => { + jest.useFakeTimers(); + const harness = setup(false); + const queued: FrameRequestCallback[] = []; + (window.requestAnimationFrame as jest.Mock).mockImplementation((callback) => { + queued.push(callback); + return queued.length; + }); + + const applyState = jest.fn(); + kickDrawerAnimation(true, applyState); + while (queued.length) { + queued.shift()?.(0); + } + + expect(applyState).toHaveBeenCalledTimes(1); + jest.runAllTimers(); + jest.useRealTimers(); + harness.unmount(); + }); +}); + describe('findHorizontalScrollBlocker', () => { const scroller = (scrollLeft: number) => { const el = document.createElement('div'); diff --git a/client/src/hooks/Nav/__tests__/useSidebarToggle.spec.tsx b/client/src/hooks/Nav/__tests__/useSidebarToggle.spec.tsx new file mode 100644 index 0000000000..977e0cebc0 --- /dev/null +++ b/client/src/hooks/Nav/__tests__/useSidebarToggle.spec.tsx @@ -0,0 +1,177 @@ +import React from 'react'; +import { RecoilRoot, useRecoilValue } from 'recoil'; +import { renderHook, act } from '@testing-library/react'; +import { MOBILE_DRAWER_ID } from '~/components/UnifiedSidebar/constants'; +import useSidebarToggle from '../useSidebarToggle'; +import useDrawerSwipe from '../useDrawerSwipe'; +import store from '~/store'; + +/** + * Exercises the real seam: useSidebarToggle must start the drawer slide + * through the swipe hook's animator BEFORE the Recoil flip commits, defer + * follow-up work past the slide, and fall back to immediate application when + * the animator is not active (desktop). + */ +const setup = (enabled: boolean, rafMode: 'sync' | 'captured' = 'sync') => { + const pane = document.createElement('div'); + const drawer = document.createElement('div'); + drawer.id = MOBILE_DRAWER_ID; + Object.defineProperty(drawer, 'clientWidth', { value: 375, configurable: true }); + document.body.append(pane, drawer); + + jest.spyOn(window, 'matchMedia').mockReturnValue({ + matches: false, + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + } as unknown as MediaQueryList); + const queued: FrameRequestCallback[] = []; + jest + .spyOn(window, 'requestAnimationFrame') + .mockImplementation((callback: FrameRequestCallback) => { + if (rafMode === 'sync') { + callback(0); + return 0; + } + queued.push(callback); + return queued.length; + }); + const flushFrames = () => { + while (queued.length) { + queued.shift()?.(0); + } + }; + + const paneRef = { current: pane }; + /** The atom defaults to true (desktop); every scenario here starts from a + * CLOSED mobile drawer, so seed it closed. */ + const wrapper = ({ children }: { children: React.ReactNode }) => ( + set(store.sidebarExpanded, false)}> + {children} + + ); + const rendered = renderHook( + () => { + useDrawerSwipe({ paneRef, enabled, open: false, onOpenChange: jest.fn() }); + return { + toggle: useSidebarToggle(), + expanded: useRecoilValue(store.sidebarExpanded), + }; + }, + { wrapper }, + ); + + return { + ...rendered, + drawer, + flushFrames, + cleanup: () => { + rendered.unmount(); + pane.remove(); + drawer.remove(); + }, + }; +}; + +describe('useSidebarToggle', () => { + afterEach(() => { + jest.restoreAllMocks(); + document.getElementById(MOBILE_DRAWER_ID)?.remove(); + /** Committed flips persist through the atom's localStorage effect, which + * overrides the next test's `initializeState` seed. */ + localStorage.clear(); + }); + + it('starts the drawer slide and reports the slide mode (mobile)', () => { + jest.useFakeTimers(); + const harness = setup(true); + + let mode = ''; + act(() => { + mode = harness.result.current.toggle.setSidebarOpen(true); + }); + + expect(mode).toBe('slide'); + expect(harness.drawer.style.transform).toBe('translate3d(0, 0, 0)'); + expect(harness.result.current.expanded).toBe(true); + jest.useRealTimers(); + harness.cleanup(); + }); + + it('flips state immediately without animating when the hook is disabled (desktop)', () => { + const harness = setup(false); + + let mode = ''; + act(() => { + mode = harness.result.current.toggle.setSidebarOpen(true); + }); + + expect(mode).toBe('none'); + expect(harness.drawer.style.transform).toBe(''); + expect(harness.result.current.expanded).toBe(true); + harness.cleanup(); + }); + + it('defers afterSlide past the slide frames on mobile', () => { + const harness = setup(true, 'captured'); + + const afterSlide = jest.fn(); + act(() => { + harness.result.current.toggle.setSidebarOpen(true, afterSlide); + }); + expect(afterSlide).not.toHaveBeenCalled(); + + act(() => { + harness.flushFrames(); + }); + expect(afterSlide).toHaveBeenCalledTimes(1); + harness.cleanup(); + }); + + it('runs afterSlide immediately on desktop', () => { + const harness = setup(false); + + const afterSlide = jest.fn(); + act(() => { + harness.result.current.toggle.setSidebarOpen(true, afterSlide); + }); + + expect(afterSlide).toHaveBeenCalledTimes(1); + harness.cleanup(); + }); + + /** The user action carried by afterSlide (navigation, focus) must survive + * an animator teardown mid-deferral โ€” only the sidebar flip goes stale. */ + it('still runs afterSlide when the animator is torn down mid-deferral', () => { + const harness = setup(true, 'captured'); + + const afterSlide = jest.fn(); + act(() => { + harness.result.current.toggle.setSidebarOpen(true, afterSlide); + }); + harness.rerender(); + harness.cleanup(); + act(() => { + harness.flushFrames(); + }); + + expect(afterSlide).toHaveBeenCalledTimes(1); + }); + + /** Two presses inside the deferral window must alternate โ€” the second + * inverts the pending target, not the still-uncommitted atom value. */ + it('alternates rapid toggles instead of piling onto the same direction', () => { + const harness = setup(true, 'captured'); + + act(() => { + harness.result.current.toggle.toggleSidebar(); + harness.result.current.toggle.toggleSidebar(); + }); + act(() => { + harness.flushFrames(); + }); + + expect(harness.result.current.expanded).toBe(false); + expect(harness.drawer.style.transform).toBe('translate3d(-100%, 0, 0)'); + harness.cleanup(); + }); +}); diff --git a/client/src/hooks/Nav/useDrawerSwipe.ts b/client/src/hooks/Nav/useDrawerSwipe.ts index 6e59a9006a..7ffb449780 100644 --- a/client/src/hooks/Nav/useDrawerSwipe.ts +++ b/client/src/hooks/Nav/useDrawerSwipe.ts @@ -23,6 +23,82 @@ const SETTLE_BUFFER_MS = 80; /** Surfaces where a horizontal drag means selection or caret work, never navigation. */ const TEXT_SURFACE_SELECTOR = 'textarea, input, select, [contenteditable="true"]'; +let drawerAnimator: ((next: boolean) => void) | null = null; + +/** How a kicked toggle was handled: no animator (desktop/logged out โ€” state + * applied immediately), reduced-motion snap (applied immediately, no slide + * frames), or a deferred slide. */ +export type DrawerKickMode = 'none' | 'snap' | 'slide'; + +/** Target of a slide whose state flip is still deferred, or null when no + * flip is pending. Lets rapid toggles invert the LATEST intent instead of + * the stale committed atom value. */ +let pendingFlipTarget: boolean | null = null; + +export function getPendingDrawerFlip(): boolean | null { + return pendingFlipTarget; +} + +/** + * Starts the drawer/pane slide imperatively and owns WHEN the caller's state + * flip runs. Wrapping the flip in `startTransition` is not enough: Recoil + * notifies subscribers outside the transition scope, so the commit โ€” which a + * large conversation stretches to hundreds of ms of context re-renders plus + * the pane's `inert` recalc โ€” flushes synchronously inside the tap's task and + * delays the slide's first frame (measured ~250ms on a 60-message thread at + * 4ร— throttle). + * + * So: start the slide imperatively, then run `applyState` three frames + * later โ€” the animator writes the target transforms in frame 1, frame 2 + * paints the first interpolated position, and by frame 3 the transform + * animation is compositor-driven (`willChange` is set in the tap's task), so + * the commit lands mid-slide without stuttering or delaying it. When the + * swipe hook is not active โ€” desktop, logged out โ€” `applyState` runs + * immediately and React animates the layout as before. + * + * Returns how the toggle was handled, so callers can keep affordances off + * the paths where they do not apply โ€” e.g. the desktop focus timer only + * runs on 'none', and follow-up work defers only on 'slide'. + */ +export function kickDrawerAnimation(next: boolean, applyState: () => void): DrawerKickMode { + const animator = drawerAnimator; + if (animator == null) { + applyState(); + return 'none'; + } + animator(next); + /** Reduced motion snaps โ€” there are no slide frames to protect, and + * deferring would let a slow-frame device outrun the 80ms window that + * restores transitions, turning the snap back into a full slide. The + * urgent commit runs inside this same task, which the restore timer + * cannot preempt. */ + if (window.matchMedia('(prefers-reduced-motion: reduce)').matches) { + applyState(); + return 'snap'; + } + pendingFlipTarget = next; + requestAnimationFrame(() => { + requestAnimationFrame(() => { + requestAnimationFrame(() => { + /** Applies only while still the LATEST un-superseded intent. A + * retarget hands the state to its own deferred flip โ€” this one + * transiently committing the old direction would re-register the + * animator and get the newer flip judged stale. Teardown and + * re-registration (breakpoint cross, logout, route change) clear + * the pending target in the effect cleanup, so stale flips drop + * out here as well instead of toggling the DESKTOP sidebar or + * persisting drawer state past logout. */ + if (pendingFlipTarget !== next) { + return; + } + pendingFlipTarget = null; + applyState(); + }); + }); + }); + return 'slide'; +} + /** * Walks from `start` up to `boundary` looking for a horizontal scroller that * can still consume a pan in `direction` (1 = finger moving right, which @@ -135,8 +211,10 @@ export default function useDrawerSwipe({ onOpenChange, }: DrawerSwipeOptions) { const gestureRef = useRef(null); + /** `id` is null while a kicked settle waits for its staged frame โ€” the + * release deadline runs on the frame clock, not the wall clock. */ const settleRef = useRef<{ - id: ReturnType; + id: ReturnType | null; target: boolean; drawer: HTMLElement; pane: HTMLElement; @@ -152,7 +230,9 @@ export default function useDrawerSwipe({ * AGREES with `open` keeps running so its handoff finishes undisturbed. */ const pending = settleRef.current; if (pending != null && (!enabled || pending.target !== open)) { - clearTimeout(pending.id); + if (pending.id != null) { + clearTimeout(pending.id); + } settleRef.current = null; releaseInlineStyles(pending.drawer, pending.pane, enabled && open); } @@ -187,6 +267,16 @@ export default function useDrawerSwipe({ gestureRef.current = null; }; + /** Arms the inline-style handoff: after the shared transition has played + * out, every transient property returns to what React/classes render. */ + const scheduleRelease = (drawerEl: HTMLElement, paneEl: HTMLElement, next: boolean) => { + const id = setTimeout(() => { + settleRef.current = null; + releaseInlineStyles(drawerEl, paneEl, next); + }, TRANSITION_MS + SETTLE_BUFFER_MS); + settleRef.current = { id, target: next, drawer: drawerEl, pane: paneEl }; + }; + /** Animates both elements to `next`, flips state, then returns ownership * of every inline property to what React/classes render for that state. */ const settle = (gesture: Gesture, next: boolean) => { @@ -215,18 +305,79 @@ export default function useDrawerSwipe({ if (next !== open) { onOpenChangeRef.current(next); } - const id = setTimeout(() => { - settleRef.current = null; - drawerEl.style.transform = ''; - drawerEl.style.willChange = ''; - drawerEl.style.transition = SIDEBAR_TRANSITION; - paneEl.style.transform = next ? 'translateX(100%)' : 'none'; - paneEl.style.willChange = ''; - paneEl.style.transition = SIDEBAR_TRANSITION; - }, TRANSITION_MS + SETTLE_BUFFER_MS); - settleRef.current = { id, target: next, drawer: drawerEl, pane: paneEl }; + scheduleRelease(drawerEl, paneEl, next); }; + /** Imperative slide for button/keyboard toggles: same transforms and + * release lifecycle as a gesture settle, but the CALLER owns the state + * flip โ€” nothing here calls `onOpenChange`. A drag in progress owns the + * inline styles, so it wins; an in-flight settle already heading to + * `next` needs no second start. */ + const animateTo = (next: boolean) => { + /** The committed `open` goes stale while a flip is deferred โ€” a rapid + * second toggle must compare against the in-flight settle's target or + * it early-returns and the visual never retargets (the state would + * still alternate, then snap at release). */ + const effectiveTarget = settleRef.current?.target ?? open; + if (next === effectiveTarget || gestureRef.current != null) { + return; + } + const pending = settleRef.current; + if (pending != null) { + if (pending.id != null) { + clearTimeout(pending.id); + } + settleRef.current = null; + } + if (window.matchMedia('(prefers-reduced-motion: reduce)').matches) { + /** Snap: suppress the shared transition through the state flip the + * caller is about to make, then hand it back once committed. */ + drawer.style.transition = 'none'; + pane.style.transition = 'none'; + const id = setTimeout(() => { + settleRef.current = null; + drawer.style.transition = SIDEBAR_TRANSITION; + pane.style.transition = SIDEBAR_TRANSITION; + }, SETTLE_BUFFER_MS); + settleRef.current = { id, target: next, drawer, pane }; + return; + } + drawer.style.willChange = 'transform'; + pane.style.willChange = 'transform'; + /** Armed BEFORE the frame below (id: null โ€” no deadline yet) so the + * staged write can verify its target is still wanted, and so a + * touchstart in between defers to the pending settle. A retarget or + * a reconciliation cancel clears/replaces the settle and thereby + * voids the write. */ + settleRef.current = { id: null, target: next, drawer, pane }; + /** The transforms are written in a FRESH frame task, not the tap's own + * task: a transition armed inside the click task is not reliably + * created for the offscreen drawer (observed via `transitionrun` + * firing only after the state-flip commit, ~270ms late, even with a + * forced reflow in-task), while the same write from a clean task + * starts interpolating by the following frame. The reflow then pins + * the transition-start recalc to this frame instead of 2โ€“3 later. + * The release deadline starts HERE, with the transition itself โ€” a + * wall-clock deadline armed at kick time can expire before a delayed + * first frame (backgrounded tab, long stall) and abort the slide. */ + requestAnimationFrame(() => { + const armed = settleRef.current; + if (armed == null || armed.target !== next || armed.id != null) { + return; + } + drawer.style.transition = SIDEBAR_TRANSITION; + pane.style.transition = SIDEBAR_TRANSITION; + drawer.style.transform = next ? 'translate3d(0, 0, 0)' : 'translate3d(-100%, 0, 0)'; + pane.style.transform = next ? 'translateX(100%)' : 'translate3d(0, 0, 0)'; + void drawer.getBoundingClientRect(); + armed.id = setTimeout(() => { + settleRef.current = null; + releaseInlineStyles(drawer, pane, next); + }, TRANSITION_MS + SETTLE_BUFFER_MS); + }); + }; + drawerAnimator = animateTo; + const onTouchStart = (event: TouchEvent) => { if (gestureRef.current != null || settleRef.current != null || event.touches.length !== 1) { return; @@ -377,6 +528,12 @@ export default function useDrawerSwipe({ surface.addEventListener('touchend', onTouchEnd, { passive: true }); surface.addEventListener('touchcancel', onTouchCancel, { passive: true }); return () => { + drawerAnimator = null; + /** A pending intent from the torn-down world must not leak into the + * next one โ€” a desktop toggle right after a breakpoint cross has to + * invert the committed value, and the deferred flip that would have + * consumed this target now drops out of its superseded check. */ + pendingFlipTarget = null; surface.removeEventListener('touchstart', onTouchStart); surface.removeEventListener('touchmove', onTouchMove); surface.removeEventListener('touchend', onTouchEnd); @@ -395,7 +552,7 @@ export default function useDrawerSwipe({ useEffect( () => () => { - if (settleRef.current != null) { + if (settleRef.current?.id != null) { clearTimeout(settleRef.current.id); } }, diff --git a/client/src/hooks/Nav/useSidebarToggle.ts b/client/src/hooks/Nav/useSidebarToggle.ts new file mode 100644 index 0000000000..22ef467dee --- /dev/null +++ b/client/src/hooks/Nav/useSidebarToggle.ts @@ -0,0 +1,76 @@ +import { startTransition, useCallback, useMemo } from 'react'; +import { useRecoilCallback, useSetRecoilState } from 'recoil'; +import type { DrawerKickMode } from './useDrawerSwipe'; +import { getPendingDrawerFlip, kickDrawerAnimation } from './useDrawerSwipe'; +import store from '~/store'; + +export type SidebarToggle = { + /** + * Opens/closes the sidebar with the slide-first contract. `afterSlide` + * carries follow-up work โ€” conversation navigation, focus handoffs โ€” that + * would otherwise run in the tap's task and flush its commit before the + * slide's first frame. On 'slide' it runs on the same three-frame cadence + * as the state flip; otherwise immediately. + */ + setSidebarOpen: (next: boolean, afterSlide?: () => void) => DrawerKickMode; + /** + * Inverts the LATEST intent: a second press inside the deferral window + * reads the pending flip target, not the still-uncommitted atom value, so + * rapid presses alternate instead of piling onto the same direction. + */ + toggleSidebar: () => void; +}; + +/** + * The one path every sidebar open/close mutation takes. On mobile the drawer + * slide starts imperatively (see kickDrawerAnimation) and the Recoil flip is + * deferred past the first frames, so the commit โ€” hundreds of ms on a large + * conversation โ€” cannot delay first motion. Setting the atom directly + * reintroduces exactly that stall, which is why conversation selection and + * the keyboard shortcuts route through here too, not just the toggle + * buttons. On desktop the flip applies immediately and the width transition + * animates as before. + */ +export default function useSidebarToggle(): SidebarToggle { + const setSidebarExpanded = useSetRecoilState(store.sidebarExpanded); + + const setSidebarOpen = useCallback( + (next: boolean, afterSlide?: () => void) => { + const mode = kickDrawerAnimation(next, () => { + startTransition(() => { + setSidebarExpanded(next); + }); + }); + if (afterSlide != null) { + if (mode === 'slide') { + /** Same cadence as the deferred flip, but deliberately NOT gated + * on the animator surviving: the user action carried here (a + * navigation, a focus request) must not vanish because the + * breakpoint crossed mid-slide. */ + requestAnimationFrame(() => { + requestAnimationFrame(() => { + requestAnimationFrame(afterSlide); + }); + }); + } else { + afterSlide(); + } + } + return mode; + }, + [setSidebarExpanded], + ); + + const toggleSidebar = useRecoilCallback( + ({ snapshot }) => + () => { + const committed = snapshot.getLoadable(store.sidebarExpanded).valueMaybe() as + | boolean + | undefined; + setSidebarOpen(!(getPendingDrawerFlip() ?? committed ?? false)); + }, + [setSidebarOpen], + ); + + return useMemo(() => ({ setSidebarOpen, toggleSidebar }), [setSidebarOpen, toggleSidebar]); +} diff --git a/client/src/hooks/useKeyboardShortcuts.ts b/client/src/hooks/useKeyboardShortcuts.ts index dc27920d71..94a0d06b2d 100644 --- a/client/src/hooks/useKeyboardShortcuts.ts +++ b/client/src/hooks/useKeyboardShortcuts.ts @@ -15,6 +15,7 @@ import { parseBinding, } from '~/utils/shortcuts'; import { mainTextareaId, NotificationSeverity } from '~/common'; +import useSidebarToggle from '~/hooks/Nav/useSidebarToggle'; import { useArchiveConvoMutation } from '~/data-provider'; import { useHasAccess, useLocalize } from '~/hooks'; import useNewChat from '~/hooks/Chat/useNewChat'; @@ -500,7 +501,8 @@ export function useShortcutActions(): ShortcutAction[] { const routeConvoId = routeMatch?.params.conversationId ?? null; const conversation = useRecoilValue(store.conversationByIndex(0)); const isSubmitting = useRecoilValue(store.isSubmittingFamily(0)); - const [sidebarExpanded, setSidebarExpanded] = useRecoilState(store.sidebarExpanded); + const sidebarExpanded = useRecoilValue(store.sidebarExpanded); + const { setSidebarOpen, toggleSidebar } = useSidebarToggle(); const setShowShortcutsDialog = useSetRecoilState(store.showShortcutsDialog); const setIsTemporary = useSetRecoilState(store.isTemporary); const setDeleteTarget = useSetRecoilState(store.keyboardDeleteTarget); @@ -531,9 +533,9 @@ export function useShortcutActions(): ShortcutAction[] { }, []); const handleToggleSidebar = useCallback(() => { - setSidebarExpanded((prev) => !prev); + toggleSidebar(); return true; - }, [setSidebarExpanded]); + }, [toggleSidebar]); const handleOpenModelSelector = useCallback( () => clickElement('[data-testid="model-selector-button"]'), @@ -566,16 +568,23 @@ export function useShortcutActions(): ShortcutAction[] { } if (!sidebarExpanded) { - setSidebarExpanded(true); + /** The focus rides `afterSlide` + a zero timer: it must queue behind + * the deferred flip's commit (which un-inerts the drawer), where a + * fixed 350ms guess could fire into the still-inert drawer and be + * silently ignored. */ + setSidebarOpen(true, () => { + setTimeout(focusSearchInput, 0); + }); + return true; } - if (!sidebarExpanded || switchedPanel) { + if (switchedPanel) { setTimeout(focusSearchInput, 350); return true; } return focusSearchInput(); - }, [sidebarExpanded, setSidebarExpanded]); + }, [sidebarExpanded, setSidebarOpen]); const handleCopyLastResponse = useCallback(() => { return clickLastElement('[data-testid="copy-response-button"]'); @@ -796,14 +805,18 @@ export function useShortcutActions(): ShortcutAction[] { } if (!sidebarExpanded) { - setSidebarExpanded(true); - setTimeout(activatePanel, 350); + /** Queued behind the deferred flip's commit, like the search focus + * above โ€” the panel button is unreachable while the drawer is + * inert. */ + setSidebarOpen(true, () => { + setTimeout(activatePanel, 0); + }); return true; } return activatePanel(); }, - [sidebarExpanded, setSidebarExpanded], + [sidebarExpanded, setSidebarOpen], ); const handleOpenAssistants = useCallback(() => handleOpenPanel('assistants'), [handleOpenPanel]); diff --git a/client/src/routes/Root.tsx b/client/src/routes/Root.tsx index 7253f61f84..236c88faa5 100644 --- a/client/src/routes/Root.tsx +++ b/client/src/routes/Root.tsx @@ -18,7 +18,6 @@ import { UnifiedSidebar, SIDEBAR_TRANSITION } from '~/components/UnifiedSidebar' import KeyboardShortcutsDialog from '~/components/Nav/KeyboardShortcutsDialog'; import KeyboardDeleteDialog from '~/components/Nav/KeyboardDeleteDialog'; import { useUserTermsQuery, useGetStartupConfig } from '~/data-provider'; -import { CLOSE_SIDEBAR_ID } from '~/components/Chat/Menus/OpenSidebar'; import useKeyboardShortcuts from '~/hooks/useKeyboardShortcuts'; import useSidebarState from '~/hooks/Nav/useSidebarState'; import { TermsAndConditionsModal } from '~/components/ui'; @@ -47,18 +46,14 @@ export default function Root() { setExpanded: setSidebarExpanded, } = useSidebarState(); const paneRef = useRef(null); + /** Focus handoff lives in the drawer header's own expanded-effect โ€” the + * commit drives it, so every opener (button, swipe) is covered without a + * timer racing the deferred state flip. */ const handleDrawerOpenChange = useCallback( (next: boolean) => { startTransition(() => { setSidebarExpanded(next); }); - if (next) { - /** Same handoff as the OpenSidebar button: opening makes the pane - * inert, so keyboard/AT focus must land inside the drawer. */ - setTimeout(() => { - document.getElementById(CLOSE_SIDEBAR_ID)?.focus(); - }, 250); - } }, [setSidebarExpanded], ); diff --git a/packages/client/src/components/Button.spec.tsx b/packages/client/src/components/Button.spec.tsx index 145b232df3..74cd664fa8 100644 --- a/packages/client/src/components/Button.spec.tsx +++ b/packages/client/src/components/Button.spec.tsx @@ -18,6 +18,18 @@ describe('Button', () => { ); }); + it('renders the header-action toggle from semantic tokens', () => { + render(); + + expect(screen.getByRole('button', { name: 'Toggle' })).toHaveClass( + 'bg-presentation', + 'border-border-light', + 'rounded-xl', + 'duration-0', + 'hover:bg-surface-active-alt', + ); + }); + it('preserves variant geometry until a shape is explicitly selected', () => { const { rerender } = render(); const button = screen.getByRole('button', { name: 'Subtle' }); diff --git a/packages/client/src/components/Button.tsx b/packages/client/src/components/Button.tsx index be6580f747..1998ee8e26 100644 --- a/packages/client/src/components/Button.tsx +++ b/packages/client/src/components/Button.tsx @@ -17,6 +17,7 @@ type ButtonVariantOptions = | 'ghost' | 'row-action' | 'section-action' + | 'header-action' | null | undefined; size?: 'default' | 'icon' | 'icon-sm' | 'icon-xs' | 'sm' | 'lg' | 'theme' | null | undefined; @@ -49,6 +50,16 @@ const buttonVariantRecipe = cva( */ 'section-action': 'rounded-md text-text-secondary hover:bg-surface-active-alt hover:text-text-primary focus-visible:ring-inset focus-visible:ring-offset-0', + /** + * A control floating on the presentation surface โ€” the sidebar + * toggle in the chat header and its mirror in the mobile drawer + * header, so the pair reads as one persistent button across views. + * `duration-0` makes the hover fill instant: these sit over a + * scrolling gradient, where the shared color transition reads as + * lag rather than polish. + */ + 'header-action': + 'rounded-xl border border-border-light bg-presentation text-text-primary duration-0 hover:bg-surface-active-alt hover:text-text-primary', }, size: { default: 'h-10 px-4 py-2',