From e7fa54dacfcd2a23c9f1aa3a3857780ba1ee184b Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 16 Aug 2026 08:29:25 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=B1=20feat:=20Give=20the=20Mobile=20Na?= =?UTF-8?q?v=20the=20Whole=20Screen=20(#14849)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 📱 feat: Give the Mobile Nav the Whole Screen The drawer was `min(85vw, 380px)` with the 52px icon rail inside it, so the conversation list got well under half the screen while ten unlabelled glyphs held a permanent column. The drawer now takes the viewport. Neither side needs a width literal any more: the panel is `fixed`, so `w-full` is the initial containing block, and the chat pane's `translateX(100%)` is self-referential and survives rotation. The shared transition moves to a constant — the two elements must stay frame-locked or the seam shows mid-animation. Drop the rail on mobile by not rendering `ExpandedPanel` rather than branching inside it, so desktop keeps an untouched file. Its four jobs move to a drawer header (panel switcher, account, close) and a bottom bar. The switcher doubles as the drawer title, answering "where am I" and "take me elsewhere" with one control, and lists panels as labelled rows. Search and new chat were both in the top corner — the two most frequent actions in the hardest place to reach one-handed. They move to a bottom bar built as a flex footer, not an overlay, so the virtualized list shrinks around it and can never be occluded. The backdrop is gone: at full width it can never be tapped, and `Root` already marks the covered pane `inert`. That makes the header's close button the primary dismissal, so it keeps `CLOSE_SIDEBAR_ID`, which `OpenSidebar` focuses after opening. Reset the drawer closed once per mobile mount. `sidebarExpanded` persists, and at full width a stale open state would launch into the nav rather than the conversation. Conversation rows revealed their overflow menu on hover, which touch does not have, leaving it reachable only on the active row. Touch now gets a cheap always-visible trigger that mounts the real menu already open, rather than mounting six mutations per overscanned row. * 🩹 fix: Address Codex Findings on the Full-Width Mobile Nav The panel switcher was unreachable. `DropdownPopup` portals to `document.body`, where `usePopoverZIndex()` hands it 50 outside a dialog — behind the opaque full-screen drawer at 110 — so none of its destinations could be selected. Render it inside the drawer instead; nothing between the trigger and the drawer root clips overflow. The drawer's z-index moves to a named constant carrying that reasoning. Panel keyboard shortcuts stopped working on mobile. They locate a panel by its rail button, read `aria-pressed`, then click it, and the rail no longer exists — so Agents, Prompts, Memories and the rest silently no-opped on a narrow window or a tablet with a keyboard. Hidden persistent targets keep that contract without reviving the rail. Routing the shortcuts through `useActivePanel` instead would mean hoisting `ActivePanelProvider` above `SidebarChatProvider`, which exists to keep panel changes from re-running `useChatHelpers`. Only available links render, so a shortcut for a panel this endpoint lacks still correctly does nothing. The persisted-drawer reset ran after the first paint, so a reload with the drawer open showed the nav covering the app and then animated it shut — the exact state it was meant to prevent. `atomWithLocalStorage` already accepts a normalizer, so the value is corrected during atom initialization and the closed state reaches the first paint. Drops the effect entirely. Note the normalizer also rewrites the stored value, so opening the drawer on a phone leaves that browser's desktop sidebar collapsed until toggled. * 🩹 fix: Address the Second Codex Round on the Full-Width Nav The conversation row's overflow menu was unreachable on mobile for the same reason the panel switcher was: `ConvoOptions` portals to `document.body`, where `usePopoverZIndex()` gives it 50, behind the drawer at 110. It now portals only off mobile — on desktop the sidebar is in normal flow, so portaling still buys escape from the list's clipping. The touch trigger also lost its own first tap. Touch browsers focus a button mid-tap, and the row's `onFocus` sets `hasInteracted`, which swaps the trigger for `ConvoOptions` before the click can land. Moving to `pointerdown` runs the handler before the swap. Crossing into the mobile breakpoint left the drawer open. The persisted value is normalized when the atom initializes, which covers loading on a phone, but narrowing a window or rotating a tablet has no such moment and an expanded desktop sidebar became a drawer covering the app. Collapse on the transition specifically, so the initial mobile paint still comes from the normalizer rather than an effect. The new spec pins the tap contract: it fires only `pointerdown`, so a click-based handler fails it. * 🩹 fix: Address the Third Codex Round on the Full-Width Nav The touch options trigger handled only `pointerdown`, so assistive tech, voice control and keyboard activation — which dispatch `click` with no preceding pointer event — did not reach it, and the click bubbled to the row and navigated away instead. It now handles pointer, click and Enter or Space through one handler. The two paths cannot double-fire, since `pointerdown` removes the button before a click could follow. The breakpoint reset still animated. Correcting it in an effect meant the first render after crossing into mobile painted the drawer open with the conversation translated fully offscreen, then moved both back over 300ms. The closed state is now derived during the transition render itself, and the effect only commits it. That derivation has to be shared: `UnifiedSidebar` draws the drawer while `Root` translates the pane, and both read the atom independently, so either one deciding alone would disagree with the other for that frame. Both now read through `useSidebarState`. * 🩹 fix: Restore Portaling and Scope the Drawer's Close Identity Revert the mobile menus to `portal={true}`. The premise behind rendering them in place was wrong: the drawer's z-index only ranks it inside `Root`'s `relative z-0` stacking context, so it cannot occlude a popup portaled to `document.body` regardless of the values involved. `ConvoOptions` has always portaled from inside this drawer and has always worked. Rendering in place cost real breakage: the row sits under the nav's `overflow-hidden` and a virtualized list, and the drawer's transform makes it the containing block for fixed descendants, so menus near a list edge were clipped and their rename, archive and delete actions unreachable. Scope the drawer's close button to the open state. It stays mounted while closed so the drawer can slide, and a translated element still counts as visible, so anything probing for `close-sidebar-button` found a control sitting off-viewport — which is what stalled the mobile visual specs. The rail this replaced only published that id while expanded; match it, and keep the closed drawer out of the tab order. * 🩹 fix: Let an Ordinary Click Open the Conversation Menu The trigger committed on `pointerdown`, so beginning a vertical scroll on an ellipsis opened that conversation's menu before the browser could tell a tap from a swipe. That handler only existed to beat a race of our own making: `hasInteracted` is hover- and focus-driven, which is meaningful on a pointer device but not on touch, where focus lands mid-tap — swapping the trigger for `ConvoOptions` while the finger was still down. Key the swap to the menu's own state on touch and the race disappears, so a plain click suffices. The browser already withholds a click until a press resolves as a tap, and synthesises one for keyboard and assistive-technology activation, which the `pointerdown` path had to special-case separately. Also correct the drawer z-index comment, which described the opposite of the layering the code settled on and would have led the next caller back into the clipping bug, and restore `aria-keyshortcuts` on the new-chat button so its binding stays discoverable. * 🩹 fix: Let Escape Leave the Menu Before the Drawer Menus opened from the drawer portal out of it, so their Escape still reached the drawer's document listener and collapsed the whole thing rather than the level the user meant to leave. Those menus unmount when closed, so their presence in the document is the signal to stand down. Also restore the toggle binding on the close control. It is the only close affordance while the drawer is open — the header's `OpenSidebar` is inside the inert, translated chat pane — so assistive technology had no way to discover the shortcut from there. * 🩹 fix: Only Treat an Open Menu as Reason to Keep the Drawer The Escape guard matched any `[role="menu"]` in the document, but not every menu unmounts when closed — the account menu stays mounted and merely `hidden`. Once it had lazily loaded, a closed menu would have suppressed Escape for the drawer permanently. Match only menus that are actually open. * ✅ test: Pin the Ariakit Closed-Menu Contract The drawer's Escape guard stands down only for menus that are actually open, which depends on Ariakit keeping a closed menu mounted and marking it `hidden` rather than unmounting it — the account menu behaves this way and would otherwise suppress Escape for the drawer permanently. Exercised against the real library rather than a mock, so a change in that behaviour fails here and points at the guard. * 🩹 fix: Keep the Row's Menu Mounted Once It Has Been Opened Keying the swap to `isPopoverActive` meant dismissing the menu unmounted `ConvoOptions` immediately, destroying Ariakit's own button — its final-focus target — mid-close. The lightweight trigger that took its place is a different node and never received focus, so a keyboard or assistive-technology user was dropped to the document instead of returning to the control they opened. Once a row's menu has been opened, keep the real one. Rows the user never touched still mount nothing, which was the reason for the trigger. * 🩹 fix: Complete the Retained-Menu Path for Touch Rows Three gaps in the retained-menu approach, all reachable. `hasOpenedMenu` was only set by the touch trigger, but the active row already renders the real menu and never passes through it. A row opened while active and later demoted would swap its focused button for a new node and drop focus — the same defect the retention was added to prevent. Recorded on every opening instead. The retained button then stayed invisible: `ConvoOptions` reveals its trigger on hover or focus when the row is neither active nor open, and touch has neither, so an interacted row was left with an invisible hit target. Kept visible on small screens. The touch trigger also restated the shared control's sizing, rounding and text treatment by hand, losing the focus ring, transitions and disabled handling that come with it. Composed from `Button` with only the local sizing retained. * ♻️ refactor: Give the Row's Overflow Control One Owner Five review rounds in this file each fixed something the previous fix introduced — trigger swap, activation path, scroll-versus-tap, focus return, retention completeness. The cause was structural rather than any one mistake: two controls can represent a row's menu, `ConvoOptions` and the cheap placeholder that stands in for it, and the rules they must agree on were spread across four separate expressions and a button, so each repair taught one of them something the other never learned. `ConvoActions` now settles them together — which control renders, when the real one becomes permanent, how it stays visible without hover, and how activation is claimed — with the reasoning for each recorded where the decision is made, including why a plain click is the right event and what breaks if a press is claimed earlier. Behaviour is unchanged; this is the same set of rules in one place. `Convo` keeps the open state, which it needs to suppress row navigation, and now passes a single `onOpenChange` rather than driving the swap itself. * 🩹 fix: Reveal the Real Menu Trigger on Touch and Recheck the Drawer Default The conversation menu has two triggers — the shift-held variant and the Ariakit button used the rest of the time — and only the first was taught to stay visible without hover. The second restated the same class string by hand instead of sharing it, so the earlier fix silently missed the trigger that actually matters. It now composes the shared string, which is why the two could disagree at all. Separately, the sidebar default is captured when the store module is evaluated, and `atomWithLocalStorage` only ran its normalizer when a saved value existed. A first visit that loaded wide and narrowed before the app mounted — a login screen being resized — therefore kept `true` with nothing to correct it, and `useMediaQuery` now resolving on the first render means the breakpoint guard sees no transition either. Normalize the default at initialization as well; callers without a normalizer get the identity function, so nothing else changes. * ✅ test: Cover the Normalized Default in `atomWithLocalStorage` Normalizing the default reaches every atom built with the helper, so the cases worth pinning are the ones where a normalizer exists and could move an untouched default: no normalizer, one that accepts the default — the shape the speech-engine atoms have — one that rejects it, and a persisted value, which must still be normalized as before. * 🩹 fix: Carry the Search Text Across a Breakpoint Change Moving search into the drawer's bottom bar left it mounted in two places — the list on a pointer device, the bottom bar on touch — so crossing the breakpoint mid-search destroys one instance and builds another. The field seeded its text to an empty string and never read the stored query, so the results stayed filtered by a term the box no longer showed, with no clear affordance to undo it. Seeded from the query instead, along with the clear button's state. * 💄 style: Settle the Drawer's Panel Switcher and Bookmark Filter The switcher's chevron trailed the panel name instead of sitting on the edge, so the control read as text with an arrow stuck to it rather than a menu spanning the header. The label now takes the slack. Moving search to the bottom bar also left the bookmark filter alone on a row of its own above the list, with nothing to sit beside. It moves next to the Chats heading, matching the Projects heading that already keeps its actions there, and the row disappears on mobile rather than lingering with one icon in it. `ChatsHeader` gains a trailing slot for that, so section actions have a home instead of a floating row. * 💄 style: Match the Bookmark Filter to the Section Actions The bookmark control was built for the row it used to share with the search field — 36px, `rounded-lg`, a larger icon — so beside a section heading it read as a different kind of control to the Projects actions sitting one row above it. Both now draw from one recipe, at every width rather than only where the move exposed it, so the two headings cannot drift apart in size, radius or hover treatment. * 🩹 fix: Cancel the Search Debounce the Field Leaves Behind The debounced commit writes to shared search state, so a pending timer outlives the instance that scheduled it. Mounting the field in two places made that reachable: crossing the breakpoint mid-keystroke destroys the list's field and builds the bottom bar's, and the departing timer would then reinstate a query the replacement had already edited or cleared. Clearing the field had the same hole within a single instance. Cancelling needs a debounce that is stable for the field's lifetime. A memo rebuilt on dependency changes leaves the previous instance's timer running past the cancel meant to stop it, and cancelling on that rebuild discards live keystrokes instead — so the handlers are read through a ref and the debounce is built once. Renames the spec, since hydrating the arriving instance and silencing the departing one are two halves of the same remount. * 🩹 fix: Hand the Uncommitted Query to the Arriving Search Field Cancelling the departing field's debounce stopped it overwriting a query the replacement had edited, but it also stranded the simpler case: a user who crosses the breakpoint and then just stops typing. The commit that would have published their query died with the instance that scheduled it, so the arriving field showed text the list was not filtered by and `isTyping` was never cleared — the loading state has no other way out while `debouncedQuery` and `query` disagree. The arriving field now takes the handoff, scheduling the commit itself when it mounts with an uncommitted query. Reading that at first render keeps it to the moment of the swap, so a real edit still wins. * 💄 style: Give Section Actions a Home in the Button Recipe The two sidebar headings shared their icon-button appearance through a feature-local class string, which is the shallow wrapper the styling rules warn about: sizing, radius, hover and focus ring are reusable appearance decisions, so they belong to the shared primitive where future theme and accessibility work will reach them. `sectionAction` and an `iconSm` size carry that recipe now, and the call sites keep only their layout. The drawer's panel switcher gets the same treatment for a sharper reason than consistency: its hand-written class string had no focus-visible state at all, so keyboard focus on the drawer's primary navigation control was invisible. Composing the shared ghost recipe restores the ring and transition, leaving only the row-filling layout local. `buttonVariants` returns unmerged recipe output, so every call site wraps it in `cn` — a spec pins that, since forgetting it silently reinstates whichever base utility the variant meant to override. * 🩹 fix: Publish the Search Field's Pending Query When It Leaves Cancelling on unmount assumed a replacement field would always arrive to inherit the query, so the fix grew a second mechanism to hand it over. The bottom bar disproves the assumption: switching panels drops the search entirely, leaving `query` set, `debouncedQuery` stale and `isTyping` on with nothing left to clear it. Flushing replaces both mechanisms. It publishes the pending commit rather than discarding it, so a field that leaves without a successor still settles the state it changed. And because a flush is synchronous with the unmount, it lands before any edit the replacement makes — which is what the cancel was for, so nothing is given up. Also normalizes the default on the parse-error path in `atomWithLocalStorage`: unparseable storage falls back to the same module-time default as a missing key, and only the missing-key path was re-checking it against the current viewport. --- .../Conversations/Conversations.tsx | 10 +- client/src/components/Conversations/Convo.tsx | 19 +- .../components/Conversations/ConvoActions.tsx | 104 +++++++++ .../ConvoOptions/ConvoOptions.tsx | 22 +- .../Conversations/ProjectsSection.tsx | 8 +- .../__tests__/Convo.touch.spec.tsx | 151 ++++++++++++++ .../components/Nav/Bookmarks/BookmarkNav.tsx | 15 +- client/src/components/Nav/SearchBar.tsx | 78 ++++--- .../Nav/__tests__/SearchBar.remount.spec.tsx | 197 ++++++++++++++++++ .../UnifiedSidebar/ConversationsSection.tsx | 28 ++- .../UnifiedSidebar/UnifiedSidebar.tsx | 100 +++++---- .../__tests__/ariakitMenuHidden.spec.tsx | 40 ++++ .../__tests__/drawerEscape.spec.ts | 46 ++++ .../components/UnifiedSidebar/constants.ts | 22 ++ client/src/components/UnifiedSidebar/index.ts | 1 + .../UnifiedSidebar/mobile/BottomBar.tsx | 68 ++++++ .../UnifiedSidebar/mobile/Header.tsx | 60 ++++++ .../UnifiedSidebar/mobile/ShortcutTargets.tsx | 40 ++++ .../UnifiedSidebar/mobile/Switcher.tsx | 88 ++++++++ .../mobile/__tests__/Header.spec.tsx | 73 +++++++ .../mobile/__tests__/ShortcutTargets.spec.tsx | 63 ++++++ .../components/UnifiedSidebar/mobile/index.ts | 4 + client/src/hooks/Nav/useSidebarState.ts | 48 +++++ client/src/routes/Root.tsx | 16 +- .../__tests__/atomWithLocalStorage.spec.ts | 72 +++++++ .../store/__tests__/sidebarExpanded.spec.ts | 88 ++++++++ client/src/store/settings.ts | 14 +- client/src/store/utils.ts | 21 +- .../client/src/components/Button.spec.tsx | 31 +++ packages/client/src/components/Button.tsx | 12 +- 30 files changed, 1415 insertions(+), 124 deletions(-) create mode 100644 client/src/components/Conversations/ConvoActions.tsx create mode 100644 client/src/components/Conversations/__tests__/Convo.touch.spec.tsx create mode 100644 client/src/components/Nav/__tests__/SearchBar.remount.spec.tsx create mode 100644 client/src/components/UnifiedSidebar/__tests__/ariakitMenuHidden.spec.tsx create mode 100644 client/src/components/UnifiedSidebar/__tests__/drawerEscape.spec.ts create mode 100644 client/src/components/UnifiedSidebar/constants.ts create mode 100644 client/src/components/UnifiedSidebar/mobile/BottomBar.tsx create mode 100644 client/src/components/UnifiedSidebar/mobile/Header.tsx create mode 100644 client/src/components/UnifiedSidebar/mobile/ShortcutTargets.tsx create mode 100644 client/src/components/UnifiedSidebar/mobile/Switcher.tsx create mode 100644 client/src/components/UnifiedSidebar/mobile/__tests__/Header.spec.tsx create mode 100644 client/src/components/UnifiedSidebar/mobile/__tests__/ShortcutTargets.spec.tsx create mode 100644 client/src/components/UnifiedSidebar/mobile/index.ts create mode 100644 client/src/hooks/Nav/useSidebarState.ts create mode 100644 client/src/store/__tests__/atomWithLocalStorage.spec.ts create mode 100644 client/src/store/__tests__/sidebarExpanded.spec.ts diff --git a/client/src/components/Conversations/Conversations.tsx b/client/src/components/Conversations/Conversations.tsx index eb3a981334..b702ceeb2e 100644 --- a/client/src/components/Conversations/Conversations.tsx +++ b/client/src/components/Conversations/Conversations.tsx @@ -5,6 +5,7 @@ import { ChevronDown } from 'lucide-react'; import { Spinner, useMediaQuery } from '@librechat/client'; import { List, CellMeasurer, CellMeasurerCache } from 'react-virtualized'; import type { TConversation } from 'librechat-data-provider'; +import type { ReactNode } from 'react'; import { useLocalize, TranslationKeys, @@ -39,6 +40,8 @@ interface ConversationsProps { isChatsExpanded: boolean; setIsChatsExpanded: (expanded: boolean) => void; showFavorites?: boolean; + /** Actions for the Chats header, alongside the Projects header's own. */ + chatsHeaderTrailing?: ReactNode; } interface MeasuredRowProps { @@ -86,10 +89,12 @@ LoadingSpinner.displayName = 'LoadingSpinner'; interface ChatsHeaderProps { isExpanded: boolean; onToggle: () => void; + /** Section-scoped actions, mirroring the Projects header. */ + trailing?: ReactNode; } /** Collapsible header for the Chats section */ -const ChatsHeader: FC = memo(({ isExpanded, onToggle }) => { +const ChatsHeader: FC = memo(({ isExpanded, onToggle, trailing }) => { const localize = useLocalize(); return ( @@ -109,6 +114,7 @@ const ChatsHeader: FC = memo(({ isExpanded, onToggle }) => { aria-hidden="true" /> + {trailing} ); }); @@ -162,6 +168,7 @@ const Conversations: FC = ({ isChatsExpanded, setIsChatsExpanded, showFavorites = true, + chatsHeaderTrailing, }) => { const localize = useLocalize(); const search = useRecoilValue(store.search); @@ -435,6 +442,7 @@ const Conversations: FC = ({ setIsChatsExpanded(!isChatsExpanded)} + trailing={chatsHeaderTrailing} /> {isSearchLoading ? ( diff --git a/client/src/components/Conversations/Convo.tsx b/client/src/components/Conversations/Convo.tsx index 7d096897dd..074cec5d62 100644 --- a/client/src/components/Conversations/Convo.tsx +++ b/client/src/components/Conversations/Convo.tsx @@ -10,7 +10,7 @@ import { useNavigateToConvo, useLocalize, useShiftKey } from '~/hooks'; import ConversationEndpointIcon from './ConversationEndpointIcon'; import { areConversationRenderPropsEqual } from './utils'; import { NotificationSeverity } from '~/common'; -import { ConvoOptions } from './ConvoOptions'; +import ConvoActions from './ConvoActions'; import RenameForm from './RenameForm'; import { cn, logger } from '~/utils'; import ConvoLink from './ConvoLink'; @@ -179,7 +179,7 @@ function Conversation({ conversationId, chatProjectId: conversation.chatProjectId, isPopoverActive, - setIsPopoverActive: handlePopoverOpenChange, + onOpenChange: handlePopoverOpenChange, isShiftHeld: isActiveConvo ? isShiftHeld : false, }; @@ -193,7 +193,8 @@ function Conversation({ 'pointer-events-none max-w-0 scale-x-0 opacity-0 group-focus-within:pointer-events-auto group-focus-within:max-w-[60px] group-focus-within:scale-x-100 group-focus-within:opacity-100 group-hover:pointer-events-auto group-hover:max-w-[60px] group-hover:scale-x-100 group-hover:opacity-100'; if (isGenerating) { actionVisibilityClassName = 'pointer-events-none w-5 scale-x-100 opacity-100'; - } else if (isPopoverActive || isActiveConvo) { + } else if (isPopoverActive || isActiveConvo || isSmallScreen) { + /** Touch has no hover, so a reveal-on-hover menu is unreachable there. */ actionVisibilityClassName = 'pointer-events-auto scale-x-100 opacity-100'; } @@ -201,13 +202,15 @@ function Conversation({ if (!isGenerating && !isPopoverActive && isActiveConvo && isShiftHeld) { actionWidthClassName = 'max-w-[60px]'; } else if (!isGenerating) { - actionWidthClassName = 'max-w-[28px]'; + actionWidthClassName = isSmallScreen ? 'max-w-[36px]' : 'max-w-[28px]'; } - const showConvoOptions = !renaming && (hasInteracted || isActiveConvo); - const actionContent = isGenerating - ? generatingSpinner - : showConvoOptions && ; + let actionContent: React.ReactNode = null; + if (isGenerating) { + actionContent = generatingSpinner; + } else if (!renaming) { + actionContent = ; + } return (
void; + renameHandler: (e: MouseEvent) => void; + onOpenChange: (open: boolean) => void; +}; + +/** + * Owns the row's overflow control: which control is rendered, when it is + * visible, and where focus lives across the swap between them. + * + * Two controls can represent this menu — `ConvoOptions`, which carries six + * mutations and its own menu store, and a cheap placeholder that stands in + * until the menu is wanted. Mounting the real one for every overscanned row is + * wasteful, but the two must agree about identity, visibility, focus and + * lifecycle, and splitting those decisions across the row is what made them + * disagree. They are settled together here: + * + * - **Which.** A pointer device reveals the menu on hover, so hovering is + * enough to mount the real one. Touch has no hover, so the placeholder is + * what makes the menu discoverable at all. + * - **When it sticks.** Once a row's menu has been opened, the real one stays. + * Swapping back would destroy Ariakit's button — its own final-focus target — + * and strand focus on the document, since the placeholder is a different + * node. Rows never opened still mount nothing, which is the point. + * - **Visibility.** `ConvoOptions` reveals its trigger on hover or focus; on + * touch it is told to stay visible, or an interacted row keeps only an + * invisible hit target. + * - **Activation.** A plain click. The browser withholds it until a press + * resolves as a tap rather than a scroll, and synthesises it for keyboard and + * assistive-technology activation — so no pointer-level handling is needed, + * and none should be added: claiming the press earlier is what previously + * opened menus mid-scroll. + */ +function ConvoActions({ + hasInteracted, + isActiveConvo, + isPopoverActive, + onOpenChange, + ...options +}: ConvoActionsProps) { + const localize = useLocalize(); + const isSmallScreen = useMediaQuery('(max-width: 768px)'); + const [hasOpenedMenu, setHasOpenedMenu] = useState(false); + + const handleOpenChange = (open: boolean) => { + if (open) { + setHasOpenedMenu(true); + } + onOpenChange(open); + }; + + const showMenu = isSmallScreen + ? isPopoverActive || isActiveConvo || hasOpenedMenu + : hasInteracted || isActiveConvo; + + if (showMenu) { + return ( + + ); + } + + if (!isSmallScreen) { + return null; + } + + return ( + + ); +} + +export default memo(ConvoActions); diff --git a/client/src/components/Conversations/ConvoOptions/ConvoOptions.tsx b/client/src/components/Conversations/ConvoOptions/ConvoOptions.tsx index 47d0370bb0..5aab1339e3 100644 --- a/client/src/components/Conversations/ConvoOptions/ConvoOptions.tsx +++ b/client/src/components/Conversations/ConvoOptions/ConvoOptions.tsx @@ -2,8 +2,8 @@ import { useState, useId, useRef, memo, useCallback, useMemo } from 'react'; import * as Ariakit from '@ariakit/react'; import { useQueryClient } from '@tanstack/react-query'; import { useParams, useNavigate } from 'react-router-dom'; -import { DropdownPopup, Spinner, useToastContext } from '@librechat/client'; import { QueryKeys, PermissionTypes, Permissions } from 'librechat-data-provider'; +import { DropdownPopup, Spinner, useToastContext, useMediaQuery } from '@librechat/client'; import { Ellipsis, Share2, @@ -58,6 +58,7 @@ function ConvoOptions({ }) { const localize = useLocalize(); const queryClient = useQueryClient(); + const isSmallScreen = useMediaQuery('(max-width: 768px)'); const { index } = useChatContext(); const { data: startupConfig } = useGetStartupConfig(); const { navigateToConvo } = useNavigateToConvo(index); @@ -366,7 +367,8 @@ function ConvoOptions({ const buttonClassName = cn( 'inline-flex h-7 w-7 items-center justify-center rounded-md border-none p-0 text-sm font-medium ring-ring-primary transition-all duration-200 ease-in-out focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 disabled:opacity-50', - isActiveConvo === true || isPopoverActive + /** Touch has no hover, so a reveal-on-hover trigger is simply invisible there. */ + isActiveConvo === true || isPopoverActive || isSmallScreen ? 'opacity-100' : 'opacity-0 focus:opacity-100 group-focus-within:opacity-100 group-hover:opacity-100 data-[open]:opacity-100', ); @@ -408,6 +410,14 @@ function ConvoOptions({ {announcement} ) => { e.stopPropagation(); }} diff --git a/client/src/components/Conversations/ProjectsSection.tsx b/client/src/components/Conversations/ProjectsSection.tsx index c44df0a509..5df242591c 100644 --- a/client/src/components/Conversations/ProjectsSection.tsx +++ b/client/src/components/Conversations/ProjectsSection.tsx @@ -26,6 +26,7 @@ import { TooltipAnchor, DropdownPopup, NewChatIcon, + buttonVariants, useToastContext, } from '@librechat/client'; import type { TChatProject, TConversation } from 'librechat-data-provider'; @@ -46,8 +47,11 @@ import store from '~/store'; const INLINE_CHAT_LIMIT = 8; -const iconButtonClassName = - 'flex h-7 w-7 shrink-0 items-center justify-center rounded-md text-text-secondary outline-none transition-colors hover:bg-surface-active-alt hover:text-text-primary focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-text-primary'; +/** `cn` is what resolves the base utilities this variant overrides. */ +const iconButtonClassName = cn( + buttonVariants({ variant: 'section-action', size: 'icon-xs' }), + 'shrink-0', +); function ProjectRenameDialog({ open, diff --git a/client/src/components/Conversations/__tests__/Convo.touch.spec.tsx b/client/src/components/Conversations/__tests__/Convo.touch.spec.tsx new file mode 100644 index 0000000000..72c959aaf1 --- /dev/null +++ b/client/src/components/Conversations/__tests__/Convo.touch.spec.tsx @@ -0,0 +1,151 @@ +import { render, screen, fireEvent, act } from '@testing-library/react'; +import type { TConversation } from 'librechat-data-provider'; + +let mockIsSmallScreen = true; +const mockConvoOptionsProps: { isPopoverActive: boolean }[] = []; +let mockCloseMenu: () => void = () => undefined; + +jest.mock('@librechat/client', () => ({ + useMediaQuery: () => mockIsSmallScreen, + useToastContext: () => ({ showToast: jest.fn() }), + Spinner: () =>
, + Button: ({ children, ...props }: React.ComponentProps<'button'>) => ( + + ), +})); + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => key, + useNavigateToConvo: () => ({ navigateToConvo: jest.fn() }), + useShiftKey: () => false, +})); + +jest.mock('~/data-provider', () => ({ + useGetStartupConfig: () => ({ data: { sharedLinksEnabled: false } }), + useUpdateConversationMutation: () => ({ mutateAsync: jest.fn() }), +})); + +jest.mock('react-router-dom', () => ({ + useParams: () => ({ conversationId: 'other-convo' }), +})); + +jest.mock('recoil', () => ({ + useRecoilValue: () => [], +})); + +jest.mock('~/store', () => ({ + __esModule: true, + default: { allConversationsSelector: 'allConversationsSelector' }, +})); + +jest.mock('~/utils', () => ({ + cn: (...classes: unknown[]) => classes.filter(Boolean).join(' '), + logger: { error: jest.fn() }, +})); + +jest.mock('../ConvoOptions', () => ({ + ConvoOptions: (props: { isPopoverActive: boolean; setIsPopoverActive: (o: boolean) => void }) => { + mockConvoOptionsProps.push(props); + mockCloseMenu = () => props.setIsPopoverActive(false); + return
; + }, +})); + +jest.mock('../ConversationEndpointIcon', () => ({ + __esModule: true, + default: () =>
, +})); + +jest.mock('../ConvoLink', () => ({ + __esModule: true, + default: ({ title }: { title: string }) => {title}, +})); + +jest.mock('../RenameForm', () => ({ + __esModule: true, + default: () =>
, +})); + +import Conversation from '../Convo'; + +const conversation = { + conversationId: 'convo-1', + title: 'Mobile UI redesign', +} as TConversation; + +const renderRow = () => + render(); + +describe('Conversation row on touch', () => { + beforeEach(() => { + mockConvoOptionsProps.length = 0; + mockIsSmallScreen = true; + }); + + it('offers a reachable overflow trigger without hover', () => { + renderRow(); + + expect(screen.getByTestId('convo-options-trigger')).toBeInTheDocument(); + expect(screen.queryByTestId('convo-options')).not.toBeInTheDocument(); + }); + + it('opens the menu on the very first tap', () => { + renderRow(); + + fireEvent.click(screen.getByTestId('convo-options-trigger')); + + expect(screen.getByTestId('convo-options')).toHaveAttribute('data-open', 'true'); + }); + + it('survives focus arriving before the press completes', () => { + renderRow(); + + /** + * Touch focuses the button mid-tap. The trigger must not be swapped out + * from under the finger, or the click never lands on it. + */ + fireEvent.focus(screen.getByTestId('convo-options-trigger')); + + expect(screen.getByTestId('convo-options-trigger')).toBeInTheDocument(); + + fireEvent.click(screen.getByTestId('convo-options-trigger')); + + expect(screen.getByTestId('convo-options')).toHaveAttribute('data-open', 'true'); + }); + + it('keeps the real menu mounted once dismissed, so focus has somewhere to return', () => { + renderRow(); + + fireEvent.click(screen.getByTestId('convo-options-trigger')); + expect(screen.getByTestId('convo-options')).toHaveAttribute('data-open', 'true'); + + /** + * Ariakit returns focus to its own trigger on close. Swapping back to the + * lightweight button would destroy that node mid-dismissal and strand + * focus on the document. + */ + act(() => mockCloseMenu()); + + expect(screen.getByTestId('convo-options')).toBeInTheDocument(); + expect(screen.queryByTestId('convo-options-trigger')).not.toBeInTheDocument(); + }); + + it('does not open from a press that turns into a scroll', () => { + renderRow(); + + /** + * A press that becomes a vertical swipe fires `pointerdown` but never a + * click. Committing on the earlier event opened the menu mid-scroll. + */ + fireEvent.pointerDown(screen.getByTestId('convo-options-trigger')); + + expect(screen.queryByTestId('convo-options')).not.toBeInTheDocument(); + }); + + it('leaves the desktop hover reveal alone', () => { + mockIsSmallScreen = false; + renderRow(); + + expect(screen.queryByTestId('convo-options-trigger')).not.toBeInTheDocument(); + }); +}); diff --git a/client/src/components/Nav/Bookmarks/BookmarkNav.tsx b/client/src/components/Nav/Bookmarks/BookmarkNav.tsx index 95da4f2da1..5b1058b548 100644 --- a/client/src/components/Nav/Bookmarks/BookmarkNav.tsx +++ b/client/src/components/Nav/Bookmarks/BookmarkNav.tsx @@ -1,8 +1,8 @@ import { useState, useId, useMemo, useCallback, memo } from 'react'; import * as Ariakit from '@ariakit/react'; import { CrossCircledIcon } from '@radix-ui/react-icons'; -import { DropdownPopup, TooltipAnchor } from '@librechat/client'; import { BookmarkFilledIcon, BookmarkIcon } from '@radix-ui/react-icons'; +import { DropdownPopup, TooltipAnchor, buttonVariants } from '@librechat/client'; import type { FC } from 'react'; import type * as t from '~/common'; import { useGetConversationTags } from '~/data-provider'; @@ -106,19 +106,18 @@ const BookmarkNav: FC = ({ tags, setTags }: BookmarkNavProps) id="bookmark-nav-menu-button" aria-label={buttonAriaLabel} aria-pressed={tags.length > 0} + /** Matches the Projects heading's actions — it sits beside a section heading too. */ className={cn( - 'flex items-center justify-center', - 'size-9 border-none text-text-primary hover:bg-surface-hover hover:text-text-primary', - 'rounded-lg border-none p-2 hover:bg-surface-active-alt', - 'outline-none focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring-primary', - isMenuOpen ? 'bg-surface-hover' : '', + buttonVariants({ variant: 'section-action', size: 'icon-xs' }), + 'shrink-0', + isMenuOpen && 'bg-surface-active-alt text-text-primary', )} data-testid="bookmark-menu" > {tags.length > 0 ? ( -
diff --git a/client/src/components/UnifiedSidebar/UnifiedSidebar.tsx b/client/src/components/UnifiedSidebar/UnifiedSidebar.tsx index 50db095d8b..a264ce3602 100644 --- a/client/src/components/UnifiedSidebar/UnifiedSidebar.tsx +++ b/client/src/components/UnifiedSidebar/UnifiedSidebar.tsx @@ -1,22 +1,23 @@ import { useCallback, useState, useEffect, useRef, memo, startTransition } from 'react'; -import type { ReactNode } from 'react'; -import { useRecoilState } from 'recoil'; import { useForm } from 'react-hook-form'; -import { useMediaQuery } from '@librechat/client'; +import type { ReactNode } from 'react'; import type { ChatFormValues } from '~/common'; +import { + COLLAPSED_WIDTH, + EXPANDED_MIN, + TRANSITION_MS, + EASING, + SIDEBAR_TRANSITION, + DRAWER_Z_INDEX, +} from './constants'; import { ChatContext, ChatFormProvider, ActivePanelProvider } from '~/Providers'; +import { MobileHeader, MobileBottomBar, MobileShortcutTargets } from './mobile'; import useUnifiedSidebarLinks from '~/hooks/Nav/useUnifiedSidebarLinks'; +import useSidebarState from '~/hooks/Nav/useSidebarState'; import { useChatHelpers, useLocalize } from '~/hooks'; import SidePanelNav from '~/components/SidePanel/Nav'; -import ExpandedPanel from './ExpandedPanel'; import Sidebar from './Sidebar'; import { cn } from '~/utils'; -import store from '~/store'; - -const COLLAPSED_WIDTH = 52; -const EXPANDED_MIN = 360; -const TRANSITION_MS = 300; -const EASING = 'cubic-bezier(0.2, 0, 0, 1)'; function getInitialWidth(): number { const saved = localStorage.getItem('side:width'); @@ -41,8 +42,7 @@ function SidebarChatProvider({ children }: { children: ReactNode }) { function UnifiedSidebar() { const localize = useLocalize(); - const isSmallScreen = useMediaQuery('(max-width: 768px)'); - const [expanded, setExpanded] = useRecoilState(store.sidebarExpanded); + const { isSmallScreen, expanded, setExpanded } = useSidebarState(); const [sidebarWidth, setSidebarWidth] = useState(getInitialWidth); const [isResizing, setIsResizing] = useState(false); const resizeHandlers = useRef<{ move: (e: MouseEvent) => void; up: () => void } | null>(null); @@ -124,9 +124,22 @@ function UnifiedSidebar() { return; } const handler = (e: KeyboardEvent) => { - if (e.key === 'Escape') { - handleCollapse(); + if (e.key !== 'Escape') { + return; } + /** + * Menus opened from the drawer portal out of it, so their Escape still + * reaches this listener. Dismissing the whole drawer would skip the level + * the user meant to leave. + * + * Presence alone is not the signal: not every menu unmounts when closed — + * the account menu stays mounted and merely `hidden` — so matching those + * too would suppress Escape for the drawer permanently. + */ + if (document.querySelector('[role="menu"]:not([hidden])') != null) { + return; + } + handleCollapse(); }; document.addEventListener('keydown', handler); return () => document.removeEventListener('keydown', handler); @@ -134,43 +147,28 @@ function UnifiedSidebar() { if (isSmallScreen) { return ( - <> -
- - - - - - -
-
-
- +
+ + + + + + + + +
); } diff --git a/client/src/components/UnifiedSidebar/__tests__/ariakitMenuHidden.spec.tsx b/client/src/components/UnifiedSidebar/__tests__/ariakitMenuHidden.spec.tsx new file mode 100644 index 0000000000..2faa844e7b --- /dev/null +++ b/client/src/components/UnifiedSidebar/__tests__/ariakitMenuHidden.spec.tsx @@ -0,0 +1,40 @@ +import * as Ariakit from '@ariakit/react'; +import { render, screen, fireEvent } from '@testing-library/react'; + +/** + * The drawer's Escape guard stands down only for menus that are actually open, + * which relies on Ariakit marking a closed menu `hidden` rather than unmounting + * it. That is the behaviour of the account menu, which has no `unmountOnHide` + * and would otherwise suppress Escape for the drawer permanently. + */ +function Fixture() { + return ( + + + + + + + ); +} + +const openMenu = () => document.querySelector('[role="menu"]:not([hidden])'); + +describe('Ariakit menu visibility contract', () => { + it('keeps a closed menu mounted but hidden', () => { + render(); + + expect(screen.getByTestId('menu')).toBeInTheDocument(); + expect(screen.getByTestId('menu')).toHaveAttribute('hidden'); + expect(openMenu()).toBeNull(); + }); + + it('drops the hidden attribute once opened', () => { + render(); + + fireEvent.click(screen.getByTestId('trigger')); + + expect(screen.getByTestId('menu')).not.toHaveAttribute('hidden'); + expect(openMenu()).not.toBeNull(); + }); +}); diff --git a/client/src/components/UnifiedSidebar/__tests__/drawerEscape.spec.ts b/client/src/components/UnifiedSidebar/__tests__/drawerEscape.spec.ts new file mode 100644 index 0000000000..1b6e2611f4 --- /dev/null +++ b/client/src/components/UnifiedSidebar/__tests__/drawerEscape.spec.ts @@ -0,0 +1,46 @@ +/** + * The drawer collapses on Escape, but menus opened from inside it portal out, + * so their Escape reaches the same document listener. This pins which menus + * count as "open" — the account menu stays mounted and merely `hidden` when + * closed, so matching on presence alone would suppress Escape permanently. + */ +const OPEN_MENU = '[role="menu"]:not([hidden])'; + +const addMenu = ({ hidden }: { hidden: boolean }) => { + const menu = document.createElement('div'); + menu.setAttribute('role', 'menu'); + if (hidden) { + menu.setAttribute('hidden', ''); + } + document.body.appendChild(menu); + return menu; +}; + +describe('drawer Escape guard', () => { + afterEach(() => { + document.body.innerHTML = ''; + }); + + it('stands down while a menu is open', () => { + addMenu({ hidden: false }); + + expect(document.querySelector(OPEN_MENU)).not.toBeNull(); + }); + + it('still collapses the drawer when a closed menu lingers in the DOM', () => { + addMenu({ hidden: true }); + + expect(document.querySelector(OPEN_MENU)).toBeNull(); + }); + + it('collapses the drawer when no menu exists at all', () => { + expect(document.querySelector(OPEN_MENU)).toBeNull(); + }); + + it('stands down when an open menu sits alongside a closed one', () => { + addMenu({ hidden: true }); + addMenu({ hidden: false }); + + expect(document.querySelector(OPEN_MENU)).not.toBeNull(); + }); +}); diff --git a/client/src/components/UnifiedSidebar/constants.ts b/client/src/components/UnifiedSidebar/constants.ts new file mode 100644 index 0000000000..233b523548 --- /dev/null +++ b/client/src/components/UnifiedSidebar/constants.ts @@ -0,0 +1,22 @@ +export const COLLAPSED_WIDTH = 52; +export const EXPANDED_MIN = 360; +export const TRANSITION_MS = 300; +export const EASING = 'cubic-bezier(0.2, 0, 0, 1)'; + +/** + * The drawer and the chat pane move as one object, so they must stay + * frame-locked; a second copy of this string is a visible seam mid-animation. + */ +export const SIDEBAR_TRANSITION = `transform ${TRANSITION_MS}ms ${EASING}`; + +/** + * The mobile drawer is opaque and full-screen, so it sits above the chat. + * + * This ranks the drawer only *within* `Root`'s `relative z-0` stacking + * context, so it cannot occlude anything portaled to `document.body` no + * matter the value. Menus opened from inside the drawer should therefore keep + * portaling: rendering them in place puts them under the nav's + * `overflow-hidden` and the virtualized list, and the drawer's transform makes + * it their containing block, which clips them. + */ +export const DRAWER_Z_INDEX = 110; diff --git a/client/src/components/UnifiedSidebar/index.ts b/client/src/components/UnifiedSidebar/index.ts index 0a72fbaed1..3c5039d8fc 100644 --- a/client/src/components/UnifiedSidebar/index.ts +++ b/client/src/components/UnifiedSidebar/index.ts @@ -1,2 +1,3 @@ export { default as UnifiedSidebar } from './UnifiedSidebar'; export { default as ConversationsSection } from './ConversationsSection'; +export * from './constants'; diff --git a/client/src/components/UnifiedSidebar/mobile/BottomBar.tsx b/client/src/components/UnifiedSidebar/mobile/BottomBar.tsx new file mode 100644 index 0000000000..2ace62add5 --- /dev/null +++ b/client/src/components/UnifiedSidebar/mobile/BottomBar.tsx @@ -0,0 +1,68 @@ +import { memo, useCallback } from 'react'; +import { useRecoilValue } from 'recoil'; +import { SquarePen } from 'lucide-react'; +import { Button } from '@librechat/client'; +import type { NavLink } from '~/common'; +import { useActivePanel, resolveActivePanel, DEFAULT_PANEL } from '~/Providers'; +import { useShortcutAriaKey } from '~/hooks/useKeyboardShortcuts'; +import SearchBar from '~/components/Nav/SearchBar'; +import useNewChat from '~/hooks/Chat/useNewChat'; +import { useLocalize } from '~/hooks'; +import store from '~/store'; + +/** + * Search and new chat are the two most frequent actions in the drawer, and both + * sat in the top corner — the hardest place to reach one-handed. A flex footer + * rather than an overlay, so the virtualized list shrinks around it and can + * never be occluded. + */ +function BottomBar({ links, onNewChat }: { links: NavLink[]; onNewChat: () => void }) { + const localize = useLocalize(); + const search = useRecoilValue(store.search); + const switchToHistory = useRecoilValue(store.newChatSwitchToHistory); + const { active, setActive } = useActivePanel(); + + const handleNewChat = useCallback(() => { + if (switchToHistory) { + setActive(DEFAULT_PANEL); + } + onNewChat(); + }, [switchToHistory, setActive, onNewChat]); + + const { handleNewChatClick } = useNewChat({ onNewChat: handleNewChat }); + /** The shortcut fires globally; assistive tech needs it discoverable here too. */ + const newChatAriaKey = useShortcutAriaKey('newChat'); + + /** Searching messages only means anything from the conversation list. */ + const showSearch = search.enabled === true && resolveActivePanel(active, links) === DEFAULT_PANEL; + + return ( +
+ {showSearch && ( +
+ +
+ )} + +
+ ); +} + +export default memo(BottomBar); diff --git a/client/src/components/UnifiedSidebar/mobile/Header.tsx b/client/src/components/UnifiedSidebar/mobile/Header.tsx new file mode 100644 index 0000000000..add3d6bedf --- /dev/null +++ b/client/src/components/UnifiedSidebar/mobile/Header.tsx @@ -0,0 +1,60 @@ +import { memo, lazy, Suspense } from 'react'; +import { X } from 'lucide-react'; +import { Button, Skeleton } from '@librechat/client'; +import type { NavLink } from '~/common'; +import { CLOSE_SIDEBAR_ID } from '~/components/Chat/Menus/OpenSidebar'; +import { useShortcutAriaKey } from '~/hooks/useKeyboardShortcuts'; +import { useLocalize } from '~/hooks'; +import Switcher from './Switcher'; + +const AccountSettings = lazy(() => import('~/components/Nav/AccountSettings')); + +/** + * At full width there is nothing beside the drawer left to tap, so this close + * button is the primary dismissal. It keeps `CLOSE_SIDEBAR_ID` because + * `OpenSidebar` focuses that id shortly after opening. + */ +function Header({ + links, + expanded, + onClose, +}: { + links: NavLink[]; + expanded: boolean; + onClose: () => void; +}) { + const localize = useLocalize(); + const toggleSidebarAriaKey = useShortcutAriaKey('toggleSidebar'); + + return ( +
+ + }> + + + +
+ ); +} + +export default memo(Header); diff --git a/client/src/components/UnifiedSidebar/mobile/ShortcutTargets.tsx b/client/src/components/UnifiedSidebar/mobile/ShortcutTargets.tsx new file mode 100644 index 0000000000..bd145f8745 --- /dev/null +++ b/client/src/components/UnifiedSidebar/mobile/ShortcutTargets.tsx @@ -0,0 +1,40 @@ +import { memo } from 'react'; +import type { NavLink } from '~/common'; +import { useActivePanel, resolveActivePanel } from '~/Providers'; + +/** + * The panel keyboard shortcuts locate a panel by its rail button + * (`[data-testid="nav-panel-*"]`), read `aria-pressed`, and click it. The rail + * does not exist on mobile, so those shortcuts would silently no-op on a + * narrow window or a tablet with a keyboard. + * + * These hidden buttons keep that contract intact without reviving the rail. + * Routing the shortcuts through `useActivePanel` instead would mean hoisting + * `ActivePanelProvider` above `SidebarChatProvider`, which exists specifically + * to keep panel changes from re-running `useChatHelpers`. + * + * Only available links render, so a shortcut for a panel this endpoint does not + * offer still correctly does nothing. + */ +function ShortcutTargets({ links }: { links: NavLink[] }) { + const { active, setActive } = useActivePanel(); + const activeId = resolveActivePanel(active, links); + + return ( + <> + {links.map((link) => ( + + ), + Skeleton: () =>
, +})); + +jest.mock('../Switcher', () => ({ + __esModule: true, + default: () =>
, +})); + +jest.mock('~/components/Nav/AccountSettings', () => ({ + __esModule: true, + default: () =>
, +})); + +jest.mock('~/components/Chat/Menus/OpenSidebar', () => ({ + CLOSE_SIDEBAR_ID: 'close-sidebar-button', +})); + +import Header from '../Header'; + +const links = [] as NavLink[]; + +describe('mobile drawer header', () => { + it('claims the close identity while the drawer is open', () => { + render(
); + + const close = screen.getByTestId('close-sidebar-button'); + expect(close).toHaveAttribute('id', 'close-sidebar-button'); + expect(close).toHaveAttribute('aria-expanded', 'true'); + }); + + /** + * The drawer stays mounted while closed so it can slide, and a translated + * element still counts as visible — so anything probing for the close button + * would find one sitting off-viewport and act on it. + */ + it('gives up that identity once closed', () => { + render(
); + + expect(screen.queryByTestId('close-sidebar-button')).not.toBeInTheDocument(); + expect(document.getElementById('close-sidebar-button')).toBeNull(); + }); + + /** The only close control while open, so its binding must be discoverable here. */ + it('advertises the toggle shortcut on the close control', () => { + render(
); + + expect(screen.getByTestId('close-sidebar-button')).toHaveAttribute( + 'aria-keyshortcuts', + 'Meta+Shift+S', + ); + }); + + it('keeps the closed drawer out of the tab order', () => { + render(
); + + expect(screen.getByLabelText('com_nav_close_sidebar')).toHaveAttribute('tabindex', '-1'); + }); +}); diff --git a/client/src/components/UnifiedSidebar/mobile/__tests__/ShortcutTargets.spec.tsx b/client/src/components/UnifiedSidebar/mobile/__tests__/ShortcutTargets.spec.tsx new file mode 100644 index 0000000000..68da46cd57 --- /dev/null +++ b/client/src/components/UnifiedSidebar/mobile/__tests__/ShortcutTargets.spec.tsx @@ -0,0 +1,63 @@ +import { MessagesSquare, NotebookPen } from 'lucide-react'; +import { render, screen, fireEvent } from '@testing-library/react'; +import type { NavLink } from '~/common'; +import { ActivePanelProvider } from '~/Providers/ActivePanelContext'; +import ShortcutTargets from '../ShortcutTargets'; + +const links = [ + { id: 'conversations', title: 'com_ui_chats', icon: MessagesSquare, Component: () => null }, + { id: 'prompts', title: 'com_ui_prompts', icon: NotebookPen, Component: () => null }, +] as unknown as NavLink[]; + +const renderTargets = () => + render( + + + , + ); + +describe('ShortcutTargets', () => { + beforeEach(() => localStorage.clear()); + + /** + * `useKeyboardShortcuts` locates panels by these ids, reads `aria-pressed`, + * and clicks. The mobile drawer has no rail, so this is the only thing + * keeping the panel shortcuts working there. + */ + it('exposes a persistent target for every available panel', () => { + renderTargets(); + + expect(screen.getByTestId('nav-panel-conversations')).toBeInTheDocument(); + expect(screen.getByTestId('nav-panel-prompts')).toBeInTheDocument(); + }); + + it('reports which panel is active so a shortcut can skip a no-op click', () => { + renderTargets(); + + expect(screen.getByTestId('nav-panel-conversations')).toHaveAttribute('aria-pressed', 'true'); + expect(screen.getByTestId('nav-panel-prompts')).toHaveAttribute('aria-pressed', 'false'); + }); + + it('switches the active panel when clicked', () => { + renderTargets(); + + fireEvent.click(screen.getByTestId('nav-panel-prompts')); + + expect(screen.getByTestId('nav-panel-prompts')).toHaveAttribute('aria-pressed', 'true'); + expect(localStorage.getItem('side:active-panel')).toBe('prompts'); + }); + + it('offers no target for a panel this endpoint does not have', () => { + renderTargets(); + + expect(screen.queryByTestId('nav-panel-parameters')).not.toBeInTheDocument(); + }); + + it('stays out of the tab order and the accessibility tree', () => { + renderTargets(); + + const target = screen.getByTestId('nav-panel-prompts'); + expect(target).toHaveAttribute('hidden'); + expect(target).toHaveAttribute('tabindex', '-1'); + }); +}); diff --git a/client/src/components/UnifiedSidebar/mobile/index.ts b/client/src/components/UnifiedSidebar/mobile/index.ts new file mode 100644 index 0000000000..c3eda66c43 --- /dev/null +++ b/client/src/components/UnifiedSidebar/mobile/index.ts @@ -0,0 +1,4 @@ +export { default as MobileHeader } from './Header'; +export { default as MobileBottomBar } from './BottomBar'; +export { default as MobilePanelSwitcher } from './Switcher'; +export { default as MobileShortcutTargets } from './ShortcutTargets'; diff --git a/client/src/hooks/Nav/useSidebarState.ts b/client/src/hooks/Nav/useSidebarState.ts new file mode 100644 index 0000000000..8f16c55dff --- /dev/null +++ b/client/src/hooks/Nav/useSidebarState.ts @@ -0,0 +1,48 @@ +import { useEffect, useRef } from 'react'; +import { useRecoilState } from 'recoil'; +import { useMediaQuery } from '@librechat/client'; + +import type { SetterOrUpdater } from 'recoil'; + +import store from '~/store'; + +export type SidebarState = { + isSmallScreen: boolean; + expanded: boolean; + setExpanded: SetterOrUpdater; +}; + +/** + * Shared reader for the sidebar's open state. + * + * The persisted value is normalized when the atom initializes, which covers + * loading straight onto a phone. Crossing the breakpoint afterwards — narrowing + * a window, rotating a tablet — has no such moment, and an expanded desktop + * sidebar would become a drawer covering the whole app. + * + * Correcting that in an effect would still paint one frame with the drawer open + * and the conversation translated fully offscreen, then animate both back over + * 300ms. So the closed state is *derived during the transition render* and the + * effect only commits it. Both the drawer and the content pane read through + * here so they can never disagree about that frame. + */ +export default function useSidebarState(): SidebarState { + const isSmallScreen = useMediaQuery('(max-width: 768px)'); + const [expanded, setExpanded] = useRecoilState(store.sidebarExpanded); + const wasSmallScreen = useRef(isSmallScreen); + + const enteredMobile = isSmallScreen && !wasSmallScreen.current; + + useEffect(() => { + wasSmallScreen.current = isSmallScreen; + if (enteredMobile) { + setExpanded(false); + } + }, [isSmallScreen, enteredMobile, setExpanded]); + + return { + isSmallScreen, + expanded: enteredMobile ? false : expanded, + setExpanded, + }; +} diff --git a/client/src/routes/Root.tsx b/client/src/routes/Root.tsx index 3adb6ec1d9..a3bfaa9892 100644 --- a/client/src/routes/Root.tsx +++ b/client/src/routes/Root.tsx @@ -1,7 +1,5 @@ import { useState, useEffect } from 'react'; -import { useRecoilValue } from 'recoil'; import { Outlet } from 'react-router-dom'; -import { useMediaQuery } from '@librechat/client'; import { PromptGroupsProvider, AssistantsMapContext, @@ -16,15 +14,15 @@ import { useAgentsMap, useFileMap, } from '~/hooks'; +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 useKeyboardShortcuts from '~/hooks/useKeyboardShortcuts'; -import { UnifiedSidebar } from '~/components/UnifiedSidebar'; +import useSidebarState from '~/hooks/Nav/useSidebarState'; import { TermsAndConditionsModal } from '~/components/ui'; import { useHealthCheck } from '~/data-provider'; import { Banner } from '~/components/Banners'; -import store from '~/store'; /** Isolates keyboard shortcut listeners so they only mount after auth. */ function KeyboardShortcutsProvider() { @@ -40,8 +38,8 @@ function KeyboardShortcutsProvider() { export default function Root() { const [showTerms, setShowTerms] = useState(false); const [bannerHeight, setBannerHeight] = useState(0); - const sidebarExpanded = useRecoilValue(store.sidebarExpanded); - const isSmallScreen = useMediaQuery('(max-width: 768px)'); + /** Shared with the drawer so the two agree on the breakpoint-transition frame. */ + const { isSmallScreen, expanded: sidebarExpanded } = useSidebarState(); const { isAuthenticated, logout } = useAuthContext(); @@ -90,9 +88,9 @@ export default function Root() {
diff --git a/client/src/store/__tests__/atomWithLocalStorage.spec.ts b/client/src/store/__tests__/atomWithLocalStorage.spec.ts new file mode 100644 index 0000000000..09acf29fa1 --- /dev/null +++ b/client/src/store/__tests__/atomWithLocalStorage.spec.ts @@ -0,0 +1,72 @@ +import { snapshot_UNSTABLE } from 'recoil'; +import { atomWithLocalStorage } from '../utils'; + +/** + * The helper now normalizes the default when nothing is persisted, not only a + * saved value. That reaches every atom built with it, so the cases that matter + * are the ones where a normalizer exists and could change an untouched default. + */ +describe('atomWithLocalStorage default normalization', () => { + beforeEach(() => localStorage.clear()); + + const read = (atom: ReturnType>) => + snapshot_UNSTABLE().getLoadable(atom).valueOrThrow(); + + it('leaves the default alone without a normalizer', () => { + expect(read(atomWithLocalStorage('plain-default', 'kept'))).toBe('kept'); + }); + + it('leaves the default alone when the normalizer accepts it', () => { + /** The shape the speech-engine atoms have: valid defaults pass through. */ + const atom = atomWithLocalStorage('idempotent-default', 'browser', (engine) => + engine === 'browser' ? engine : 'browser', + ); + + expect(read(atom)).toBe('browser'); + }); + + it('corrects a default the normalizer rejects', () => { + const atom = atomWithLocalStorage('rejected-default', 'legacy', (engine) => + engine === 'legacy' ? 'external' : engine, + ); + + expect(read(atom)).toBe('external'); + }); + + it('still normalizes a persisted value', () => { + localStorage.setItem('persisted-value', JSON.stringify('legacy')); + const atom = atomWithLocalStorage('persisted-value', 'browser', (engine) => + engine === 'legacy' ? 'external' : engine, + ); + + expect(read(atom)).toBe('external'); + }); + + /** + * Unparseable storage falls back to the default too, and that fallback is + * subject to the same stale-at-module-load problem as the missing-key one. + */ + describe('when the persisted value cannot be parsed', () => { + beforeEach(() => { + jest.spyOn(console, 'error').mockImplementation(() => undefined); + localStorage.setItem('corrupt-value', '{not json'); + }); + + it('falls back to the normalized default', () => { + const atom = atomWithLocalStorage('corrupt-value', 'legacy', (engine) => + engine === 'legacy' ? 'external' : engine, + ); + + expect(read(atom)).toBe('external'); + }); + + it('persists the normalized default over the unusable value', () => { + const atom = atomWithLocalStorage('corrupt-value', 'legacy', (engine) => + engine === 'legacy' ? 'external' : engine, + ); + read(atom); + + expect(localStorage.getItem('corrupt-value')).toBe(JSON.stringify('external')); + }); + }); +}); diff --git a/client/src/store/__tests__/sidebarExpanded.spec.ts b/client/src/store/__tests__/sidebarExpanded.spec.ts new file mode 100644 index 0000000000..0f0ecd5d5d --- /dev/null +++ b/client/src/store/__tests__/sidebarExpanded.spec.ts @@ -0,0 +1,88 @@ +const KEY = 'unifiedSidebarExpanded'; + +/** The shared setup defines `matchMedia` as writable, so assign over it. */ +const setViewport = (isSmall: boolean) => { + window.matchMedia = ((query: string) => ({ + matches: isSmall && query === '(max-width: 768px)', + media: query, + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + addListener: jest.fn(), + removeListener: jest.fn(), + onchange: null, + dispatchEvent: jest.fn(), + })) as unknown as typeof window.matchMedia; +}; + +/** + * The atom reads the viewport at module load, so each case needs a fresh + * import — and Recoil must come from the same registry as the reloaded atom. + */ +const readInitialValue = async (): Promise => { + const { snapshot_UNSTABLE } = await import('recoil'); + const settings = await import('../settings'); + return snapshot_UNSTABLE().getLoadable(settings.default.sidebarExpanded).valueOrThrow(); +}; + +describe('sidebarExpanded', () => { + beforeEach(() => { + jest.resetModules(); + localStorage.clear(); + }); + + it('ignores a persisted open drawer on a small viewport', async () => { + setViewport(true); + localStorage.setItem(KEY, JSON.stringify(true)); + + /** + * Normalized during initialization rather than corrected by an effect, so + * the closed state reaches the first paint — otherwise the nav covers the + * app and animates shut afterwards. + */ + expect(await readInitialValue()).toBe(false); + }); + + it('honours a persisted collapsed sidebar on a wide viewport', async () => { + setViewport(false); + localStorage.setItem(KEY, JSON.stringify(false)); + + expect(await readInitialValue()).toBe(false); + }); + + it('honours a persisted open sidebar on a wide viewport', async () => { + setViewport(false); + localStorage.setItem(KEY, JSON.stringify(true)); + + expect(await readInitialValue()).toBe(true); + }); + + it('starts closed on a small viewport with nothing persisted', async () => { + setViewport(true); + + expect(await readInitialValue()).toBe(false); + }); + + /** + * The default is captured when the module is evaluated, which on a login + * screen can be long before the app mounts. Without a persisted key there is + * no saved value to normalize, so a viewport that narrows in between would + * otherwise leave the drawer covering the first authenticated screen. + */ + it('rechecks the viewport when no value was ever persisted', async () => { + setViewport(false); + const { snapshot_UNSTABLE } = await import('recoil'); + const settings = await import('../settings'); + + setViewport(true); + + expect(snapshot_UNSTABLE().getLoadable(settings.default.sidebarExpanded).valueOrThrow()).toBe( + false, + ); + }); + + it('starts open on a wide viewport with nothing persisted', async () => { + setViewport(false); + + expect(await readInitialValue()).toBe(true); + }); +}); diff --git a/client/src/store/settings.ts b/client/src/store/settings.ts index 7a3c2d1397..9f7cd94479 100644 --- a/client/src/store/settings.ts +++ b/client/src/store/settings.ts @@ -33,12 +33,24 @@ const staticAtoms = { speechSettingsInitialized: atom({ key: 'speechSettingsInitialized', default: false }), }; +/** Read synchronously: `useMediaQuery` only resolves after the first paint. */ +function isSmallViewport(): boolean { + return typeof window !== 'undefined' && window.matchMedia('(max-width: 768px)').matches; +} + const localStorageAtoms = { // General settings autoScroll: atomWithLocalStorage('autoScroll', false), sidebarExpanded: atomWithLocalStorage( 'unifiedSidebarExpanded', - typeof window !== 'undefined' && window.matchMedia('(max-width: 768px)').matches ? false : true, + !isSmallViewport(), + /** + * The mobile drawer covers the viewport, so a persisted open state would + * launch the app into the navigation instead of the conversation. + * Normalized during atom initialization so the closed state reaches the + * first paint rather than animating shut after it. + */ + (saved) => (isSmallViewport() ? false : saved), ), enableUserMsgMarkdown: atomWithLocalStorage( LocalStorageKeys.ENABLE_USER_MSG_MARKDOWN, diff --git a/client/src/store/utils.ts b/client/src/store/utils.ts index 5551549513..ed6731520c 100644 --- a/client/src/store/utils.ts +++ b/client/src/store/utils.ts @@ -11,7 +11,22 @@ export function atomWithLocalStorage( default: defaultValue, effects_UNSTABLE: [ ({ setSelf, onSet }) => { + /** + * The default is captured when the module is evaluated, which can be + * long before anything subscribes — a login screen resized before the + * app mounts, for instance. Every path that falls back to it re-checks + * that assumption here. A caller without a normalizer gets the identity + * function, so this is inert for them. + */ + const resolveDefault = () => normalizeSavedValue(defaultValue); const savedValue = localStorage.getItem(key); + + if (savedValue === null) { + const normalizedDefault = resolveDefault(); + if (!Object.is(normalizedDefault, defaultValue)) { + setSelf(normalizedDefault); + } + } if (savedValue !== null) { try { const parsedValue = JSON.parse(savedValue) as T; @@ -25,8 +40,10 @@ export function atomWithLocalStorage( `Error parsing localStorage key "${key}", \`savedValue\`: defaultValue, error:`, e, ); - localStorage.setItem(key, JSON.stringify(defaultValue)); - setSelf(defaultValue); + /** The stored value is unusable, so the normalized default replaces it. */ + const normalizedDefault = resolveDefault(); + localStorage.setItem(key, JSON.stringify(normalizedDefault)); + setSelf(normalizedDefault); } } diff --git a/packages/client/src/components/Button.spec.tsx b/packages/client/src/components/Button.spec.tsx index e85dcd907a..145b232df3 100644 --- a/packages/client/src/components/Button.spec.tsx +++ b/packages/client/src/components/Button.spec.tsx @@ -1,6 +1,7 @@ import '@testing-library/jest-dom'; import { render, screen } from '@testing-library/react'; import { Button, buttonVariants } from './Button'; +import { cn } from '~/utils'; describe('Button', () => { it('exposes theme-owned shape and density recipes', () => { @@ -57,4 +58,34 @@ describe('Button', () => { 'hover:bg-surface-hover-alt', ); }); + + /** + * Section actions sit close enough to their heading and to each other that + * the default offset ring crosses a neighbour, so this variant has to win + * both radius and ring — and stay distinct from `row-action` above. + */ + it('gives section actions an inset ring and a tighter radius', () => { + render( + , + ); + const button = screen.getByRole('button', { name: 'Filter' }); + + expect(button).toHaveClass('size-7', 'rounded-md', 'focus-visible:ring-inset'); + expect(button).not.toHaveClass('rounded-lg', 'focus-visible:ring-offset-2'); + }); + + /** + * `buttonVariants` returns raw recipe output, so conflicting utilities from + * the base survive it. Call sites applying the recipe to a non-Button element + * have to merge it themselves, and this is what breaks if they forget. + */ + it('leaves overridden base utilities in the unmerged variant helper', () => { + const sectionAction = buttonVariants({ variant: 'section-action', size: 'icon-xs' }); + + expect(sectionAction).toContain('rounded-md'); + expect(sectionAction).toContain('rounded-lg'); + expect(cn(sectionAction)).not.toContain('rounded-lg'); + }); }); diff --git a/packages/client/src/components/Button.tsx b/packages/client/src/components/Button.tsx index 4ef1483407..be6580f747 100644 --- a/packages/client/src/components/Button.tsx +++ b/packages/client/src/components/Button.tsx @@ -16,9 +16,10 @@ type ButtonVariantOptions = | 'secondary' | 'ghost' | 'row-action' + | 'section-action' | null | undefined; - size?: 'default' | 'icon' | 'icon-sm' | 'sm' | 'lg' | 'theme' | null | undefined; + size?: 'default' | 'icon' | 'icon-sm' | 'icon-xs' | 'sm' | 'lg' | 'theme' | null | undefined; shape?: 'default' | 'theme' | null | undefined; } & ClassProp) | undefined; @@ -40,6 +41,14 @@ const buttonVariantRecipe = cva( 'row-action': 'hover:bg-surface-hover-alt hover:text-text-primary', link: 'text-text-primary underline-offset-4 hover:underline', submit: 'bg-surface-submit text-text-on-status hover:bg-surface-submit-hover', + /** + * A quiet icon action sitting beside a section heading in the sidebar. + * Unlike `row-action`, it recedes until hovered so the heading stays + * the thing being read, and its ring sits inside the control because + * these sit close enough that an offset one would cross a neighbour. + */ + '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', }, size: { default: 'h-10 px-4 py-2', @@ -47,6 +56,7 @@ const buttonVariantRecipe = cva( lg: 'h-11 rounded-lg px-8', icon: 'size-10', 'icon-sm': 'size-8 p-0', + 'icon-xs': 'size-7', theme: 'h-theme-control gap-theme-compact px-theme-normal', }, shape: {