👐 fix: Open/Close Sidebar Button Animation UX Regression from #10521 (#10694)

Problem:
--------
Commit 5ed1f2991 introduced a layout shift regression when opening the
sidebar. The UI would visibly "jump" as elements shifted right before
the animation completed. Closing the sidebar worked correctly.

Root Cause Analysis:
--------------------
The accessibility PR added a redundant `{navVisible && ...}` conditional
wrapper around the `<nav>` content inside Nav.tsx's `motion.div`. This
caused a race condition:

1. User clicks "Open Sidebar" button
2. `navVisible` state becomes `true`
3. React renders the `motion.div` AND its children simultaneously
4. The inner `{navVisible && (<nav>...)}` renders content at full width
   (320px/260px) BEFORE framer-motion applies `initial={{ width: 0 }}`
5. Brief flash of full-width content causes visible layout shift
6. Animation then starts from width: 0, but damage is done

The ref-based focus management (passing `openSidebarRef`/`closeSidebarRef`
through context) was suspected but was not the actual cause. However,
`requestAnimationFrame` focus calls during animation start could trigger
forced layout calculations, exacerbating the issue.

Solution:
---------
1. Remove redundant conditional rendering in Nav.tsx
   - The outer `{navVisible && (<motion.div>...)}` already controls
     visibility
   - The `overflow-x-hidden` class on motion.div clips content during
     animation
   - Content should always exist inside motion.div for smooth clipping

2. Replace ref-based focus with ID-based focus management
   - Refs passed through component tree can affect React's reconciliation
   - Using `document.getElementById()` decouples focus from render cycle
   - Exported `CLOSE_SIDEBAR_ID` and `OPEN_SIDEBAR_ID` constants for
     consistency

3. Delay focus until after animation completes
   - Changed from `requestAnimationFrame` to `setTimeout(..., 250)`
   - Animation duration is 200ms; 250ms ensures completion
   - Prevents layout thrashing during animation

4. Clean up prop drilling
   - Removed `openSidebarRef`/`closeSidebarRef` from Root.tsx context
   - Simplified Nav.tsx, Header.tsx, NewChat.tsx prop signatures
   - Updated ContextType to remove ref properties

Files Changed:
--------------
- client/src/routes/Root.tsx
- client/src/components/Nav/Nav.tsx
- client/src/components/Nav/NewChat.tsx
- client/src/components/Chat/Header.tsx
- client/src/components/Chat/Menus/OpenSidebar.tsx
- client/src/common/types.ts

Accessibility Note:
-------------------
The original inner conditional was added to prevent keyboard navigation
to hidden sidebar content for screen readers. This is still handled by:
- AnimatePresence unmounting the motion.div after exit animation
- The motion.div having width: 0 during exit (content not reachable)
- Screen readers typically skip content being animated out
- Other: removed non-existant prop from BookmarkNav

Testing:
--------
- Verified smooth animation when opening sidebar (no layout shift)
- Verified smooth animation when closing sidebar (unchanged)
- Verified focus transfers correctly between open/close buttons
- Verified keyboard navigation works as expected
This commit is contained in:
Danny Avila 2025-11-26 18:59:21 -05:00
parent 1b0b27b30c
commit 1d9ea7f83f
No known key found for this signature in database
GPG key ID: BF31EEB2C5CA0956
6 changed files with 57 additions and 91 deletions

View file

@ -571,8 +571,6 @@ export interface ModelItemProps {
export type ContextType = {
navVisible: boolean;
setNavVisible: React.Dispatch<React.SetStateAction<boolean>>;
openSidebarRef?: React.RefObject<HTMLButtonElement>;
closeSidebarRef?: React.RefObject<HTMLButtonElement>;
};
export interface SwitcherProps {

View file

@ -18,8 +18,7 @@ const defaultInterface = getConfigDefaults().interface;
export default function Header() {
const { data: startupConfig } = useGetStartupConfig();
const { navVisible, setNavVisible, openSidebarRef, closeSidebarRef } =
useOutletContext<ContextType>();
const { navVisible, setNavVisible } = useOutletContext<ContextType>();
const interfaceConfig = useMemo(
() => startupConfig?.interface ?? defaultInterface,
@ -52,12 +51,7 @@ export default function Header() {
transition={{ duration: 0.2 }}
key="header-buttons"
>
<OpenSidebar
ref={openSidebarRef}
setNavVisible={setNavVisible}
closeSidebarRef={closeSidebarRef}
className="max-md:hidden"
/>
<OpenSidebar setNavVisible={setNavVisible} className="max-md:hidden" />
<HeaderNewChat />
</motion.div>
)}

View file

@ -1,16 +1,19 @@
import { forwardRef } from 'react';
import { TooltipAnchor, Button, Sidebar } from '@librechat/client';
import { useLocalize } from '~/hooks';
import { cn } from '~/utils';
const OpenSidebar = forwardRef<
HTMLButtonElement,
{
setNavVisible: React.Dispatch<React.SetStateAction<boolean>>;
className?: string;
closeSidebarRef?: React.RefObject<HTMLButtonElement>;
}
>(({ setNavVisible, className, closeSidebarRef }, ref) => {
/** Element ID for the close sidebar button - used for focus management */
export const CLOSE_SIDEBAR_ID = 'close-sidebar-button';
/** Element ID for the open sidebar button - used for focus management */
export const OPEN_SIDEBAR_ID = 'open-sidebar-button';
export default function OpenSidebar({
setNavVisible,
className,
}: {
setNavVisible: React.Dispatch<React.SetStateAction<boolean>>;
className?: string;
}) {
const localize = useLocalize();
const handleClick = () => {
@ -18,9 +21,10 @@ const OpenSidebar = forwardRef<
localStorage.setItem('navVisible', JSON.stringify(!prev));
return !prev;
});
requestAnimationFrame(() => {
closeSidebarRef?.current?.focus();
});
// Delay focus until after the sidebar animation completes (200ms)
setTimeout(() => {
document.getElementById(CLOSE_SIDEBAR_ID)?.focus();
}, 250);
};
return (
@ -28,7 +32,7 @@ const OpenSidebar = forwardRef<
description={localize('com_nav_open_sidebar')}
render={
<Button
ref={ref}
id={OPEN_SIDEBAR_ID}
size="icon"
variant="outline"
data-testid="open-sidebar-button"
@ -46,8 +50,4 @@ const OpenSidebar = forwardRef<
}
/>
);
});
OpenSidebar.displayName = 'OpenSidebar';
export default OpenSidebar;
}

View file

@ -50,13 +50,9 @@ const Nav = memo(
({
navVisible,
setNavVisible,
openSidebarRef,
closeSidebarRef,
}: {
navVisible: boolean;
setNavVisible: React.Dispatch<React.SetStateAction<boolean>>;
openSidebarRef?: React.RefObject<HTMLButtonElement>;
closeSidebarRef?: React.RefObject<HTMLButtonElement>;
}) => {
const localize = useLocalize();
const { isAuthenticated } = useAuthContext();
@ -170,7 +166,7 @@ const Nav = memo(
<>
<div className="mt-1.5" />
<Suspense fallback={null}>
<BookmarkNav tags={tags} setTags={setTags} isSmallScreen={isSmallScreen} />
<BookmarkNav tags={tags} setTags={setTags} />
</Suspense>
</>
)}
@ -211,36 +207,32 @@ const Nav = memo(
>
<div className="h-full w-[320px] md:w-[260px]">
<div className="flex h-full flex-col">
{navVisible && (
<nav
id="chat-history-nav"
aria-label={localize('com_ui_chat_history')}
className="flex h-full flex-col px-2 pb-3.5 md:px-3"
>
<div className="flex flex-1 flex-col" ref={outerContainerRef}>
<MemoNewChat
subHeaders={subHeaders}
toggleNav={toggleNavVisible}
headerButtons={headerButtons}
isSmallScreen={isSmallScreen}
openSidebarRef={openSidebarRef}
closeSidebarRef={closeSidebarRef}
/>
<Conversations
conversations={conversations}
moveToTop={moveToTop}
toggleNav={itemToggleNav}
containerRef={listRef}
loadMoreConversations={loadMoreConversations}
isLoading={isFetchingNextPage || showLoading || isLoading}
isSearchLoading={isSearchLoading}
/>
</div>
<Suspense fallback={null}>
<AccountSettings />
</Suspense>
</nav>
)}
<nav
id="chat-history-nav"
aria-label={localize('com_ui_chat_history')}
className="flex h-full flex-col px-2 pb-3.5 md:px-3"
>
<div className="flex flex-1 flex-col" ref={outerContainerRef}>
<MemoNewChat
subHeaders={subHeaders}
toggleNav={toggleNavVisible}
headerButtons={headerButtons}
isSmallScreen={isSmallScreen}
/>
<Conversations
conversations={conversations}
moveToTop={moveToTop}
toggleNav={itemToggleNav}
containerRef={listRef}
loadMoreConversations={loadMoreConversations}
isLoading={isFetchingNextPage || showLoading || isLoading}
isSearchLoading={isSearchLoading}
/>
</div>
<Suspense fallback={null}>
<AccountSettings />
</Suspense>
</nav>
</div>
</div>
</motion.div>

View file

@ -3,6 +3,7 @@ import { useNavigate } from 'react-router-dom';
import { QueryKeys } from 'librechat-data-provider';
import { useQueryClient } from '@tanstack/react-query';
import { TooltipAnchor, NewChatIcon, MobileSidebar, Sidebar, Button } from '@librechat/client';
import { CLOSE_SIDEBAR_ID, OPEN_SIDEBAR_ID } from '~/components/Chat/Menus/OpenSidebar';
import { useLocalize, useNewConvo } from '~/hooks';
import { clearMessagesCache } from '~/utils';
import store from '~/store';
@ -13,16 +14,12 @@ export default function NewChat({
subHeaders,
isSmallScreen,
headerButtons,
openSidebarRef,
closeSidebarRef,
}: {
index?: number;
toggleNav: () => void;
isSmallScreen?: boolean;
subHeaders?: React.ReactNode;
headerButtons?: React.ReactNode;
openSidebarRef?: React.RefObject<HTMLButtonElement>;
closeSidebarRef?: React.RefObject<HTMLButtonElement>;
}) {
const queryClient = useQueryClient();
/** Note: this component needs an explicit index passed if using more than one */
@ -33,10 +30,11 @@ export default function NewChat({
const handleToggleNav = useCallback(() => {
toggleNav();
requestAnimationFrame(() => {
openSidebarRef?.current?.focus();
});
}, [toggleNav, openSidebarRef]);
// Delay focus until after the sidebar animation completes (200ms)
setTimeout(() => {
document.getElementById(OPEN_SIDEBAR_ID)?.focus();
}, 250);
}, [toggleNav]);
const clickHandler: React.MouseEventHandler<HTMLButtonElement> = useCallback(
(e) => {
@ -62,7 +60,7 @@ export default function NewChat({
description={localize('com_nav_close_sidebar')}
render={
<Button
ref={closeSidebarRef}
id={CLOSE_SIDEBAR_ID}
size="icon"
variant="outline"
data-testid="close-sidebar-button"

View file

@ -1,4 +1,4 @@
import React, { useState, useEffect, useRef } from 'react';
import React, { useState, useEffect } from 'react';
import { Outlet } from 'react-router-dom';
import type { ContextType } from '~/common';
import {
@ -28,8 +28,6 @@ export default function Root() {
const savedNavVisible = localStorage.getItem('navVisible');
return savedNavVisible !== null ? JSON.parse(savedNavVisible) : true;
});
const openSidebarRef = useRef<HTMLButtonElement>(null);
const closeSidebarRef = useRef<HTMLButtonElement>(null);
const { isAuthenticated, logout } = useAuthContext();
@ -75,24 +73,10 @@ export default function Root() {
<Banner onHeightChange={setBannerHeight} />
<div className="flex" style={{ height: `calc(100dvh - ${bannerHeight}px)` }}>
<div className="relative z-0 flex h-full w-full overflow-hidden">
<Nav
navVisible={navVisible}
setNavVisible={setNavVisible}
openSidebarRef={openSidebarRef}
closeSidebarRef={closeSidebarRef}
/>
<Nav navVisible={navVisible} setNavVisible={setNavVisible} />
<div className="relative flex h-full max-w-full flex-1 flex-col overflow-hidden">
<MobileNav navVisible={navVisible} setNavVisible={setNavVisible} />
<Outlet
context={
{
navVisible,
setNavVisible,
openSidebarRef,
closeSidebarRef,
} satisfies ContextType
}
/>
<Outlet context={{ navVisible, setNavVisible } satisfies ContextType} />
</div>
</div>
</div>