From e49e2644878447c9eceec45a803b74e7af59e4fb Mon Sep 17 00:00:00 2001 From: James Todaro <30529065+jtodaroii@users.noreply.github.com> Date: Thu, 20 Aug 2026 11:52:53 -0400 Subject: [PATCH] =?UTF-8?q?=E2=99=BF=20fix:=20Resolve=20axe=20Violations?= =?UTF-8?q?=20in=20Sidebar,=20Tools=20Dropdown=20and=20Footer=20(#14979)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ♿ fix: Resolve axe Violations in Sidebar, Tools Dropdown and Footer - give virtualized conversation rows the row/gridcell roles their grid and rowgroup parents require - make the conversation row a non-interactive container, moving its accessible name, aria-current and focus ring onto the title control so it no longer wraps the options button - open the tools menu non-modally and portal it into the main landmark, dropping Ariakit's injected dismiss button and keeping menu content inside a landmark - expose aria-valuenow, aria-valuemin and aria-valuemax on the sidebar resize handle - drop role="contentinfo" from the chat footer, which is never rendered outside main - scan the loaded app in a11y.spec.ts, and cover seeded conversation rows, a hovered row and the open tools menu * ♿ fix: Keep the Resize Handle's ARIA Range Valid at Every Viewport - floor the announced maximum at the aside's own min-width, so viewports where 40% falls under it no longer report a maximum below the minimum - track the viewport so the announced range follows a resize instead of a render-time snapshot --- client/src/components/Chat/Footer.tsx | 1 - .../components/Chat/Input/ToolsDropdown.tsx | 9 ++- .../Conversations/Conversations.tsx | 7 +- client/src/components/Conversations/Convo.tsx | 24 +------ .../components/Conversations/ConvoLink.tsx | 19 +++-- .../src/components/UnifiedSidebar/Sidebar.tsx | 9 +++ .../UnifiedSidebar/UnifiedSidebar.tsx | 20 ++++++ e2e/specs/a11y.spec.ts | 71 +++++++++++++++++-- .../client/src/components/DropdownPopup.tsx | 2 +- 9 files changed, 126 insertions(+), 36 deletions(-) diff --git a/client/src/components/Chat/Footer.tsx b/client/src/components/Chat/Footer.tsx index 79413c7f68..31cc0d48ee 100644 --- a/client/src/components/Chat/Footer.tsx +++ b/client/src/components/Chat/Footer.tsx @@ -91,7 +91,6 @@ function Footer({ className, startupConfig }: FooterProps) { className ?? 'absolute bottom-0 left-0 right-0 hidden items-center justify-center gap-2 px-2 py-2 text-center text-xs text-text-primary sm:flex md:px-[60px]' } - role="contentinfo" > {footerElements.map((contentRender, index) => { const isLastElement = index === footerElements.length - 1; diff --git a/client/src/components/Chat/Input/ToolsDropdown.tsx b/client/src/components/Chat/Input/ToolsDropdown.tsx index 2365a4f128..66d5684ddc 100644 --- a/client/src/components/Chat/Input/ToolsDropdown.tsx +++ b/client/src/components/Chat/Input/ToolsDropdown.tsx @@ -27,6 +27,10 @@ interface ToolsDropdownProps { disabled?: boolean; } +/** Ariakit portals to document.body by default, which puts the menu outside every landmark. + * Returning null falls back to that default. */ +const getMainLandmark = () => document.querySelector('main'); + const ToolsDropdown = ({ disabled }: ToolsDropdownProps) => { const localize = useLocalize(); const { user } = useAuthContext(); @@ -400,7 +404,10 @@ const ToolsDropdown = ({ disabled }: ToolsDropdownProps) => { menuId="tools-dropdown-menu" isOpen={isPopoverActive} setIsOpen={setIsPopoverActive} - modal={true} + modal={false} + portal={true} + portalElement={getMainLandmark} + preserveTabOrder={false} unmountOnHide={true} trigger={menuTrigger} items={dropdownItems} diff --git a/client/src/components/Conversations/Conversations.tsx b/client/src/components/Conversations/Conversations.tsx index 264968c95d..ff0a0879b0 100644 --- a/client/src/components/Conversations/Conversations.tsx +++ b/client/src/components/Conversations/Conversations.tsx @@ -53,7 +53,9 @@ interface MeasuredRowProps { children: React.ReactNode; } -/** Reusable wrapper for virtualized row measurement */ +/** Reusable wrapper for virtualized row measurement. + * The List renders role="grid" over a role="rowgroup" container, so each row carries the + * row/gridcell roles those parents require of their children. */ const MeasuredRow: FC = memo( ({ cache, rowKey, parent, index, style, children }) => ( @@ -63,8 +65,9 @@ const MeasuredRow: FC = memo( style={style} className="px-3" data-testid="convo-list-row" + role="row" > - {children} +
{children}
)} diff --git a/client/src/components/Conversations/Convo.tsx b/client/src/components/Conversations/Convo.tsx index 1dc6d91d69..0f0a15340b 100644 --- a/client/src/components/Conversations/Convo.tsx +++ b/client/src/components/Conversations/Convo.tsx @@ -222,17 +222,6 @@ function Conversation({ ? 'bg-surface-active-alt before:absolute before:bottom-1 before:left-0 before:top-1 before:w-0.5 before:rounded-full before:bg-text-primary' : 'hover:bg-surface-active-alt', )} - role="button" - tabIndex={renaming ? -1 : 0} - aria-label={ - isSharedBadgeVisible - ? localize('com_ui_conversation_label_shared', { - title: title || localize('com_ui_untitled'), - }) - : localize('com_ui_conversation_label', { - title: title || localize('com_ui_untitled'), - }) - } onMouseEnter={handleMouseEnter} onMouseLeave={handleMouseLeave} onFocus={handleMouseEnter} @@ -245,18 +234,6 @@ function Conversation({ handleNavigation(e.ctrlKey || e.metaKey); } }} - onKeyDown={(e) => { - if (renaming) { - return; - } - if (e.target !== e.currentTarget) { - return; - } - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault(); - handleNavigation(false); - } - }} style={{ cursor: renaming ? 'default' : 'pointer' }} data-testid="convo-item" > @@ -272,6 +249,7 @@ function Conversation({ void; isSmallScreen: boolean; @@ -14,6 +15,7 @@ interface ConvoLinkProps { const ConvoLink: React.FC = ({ isActiveConvo, isPopoverActive, + isSharedBadgeVisible, title, onRename, isSmallScreen, @@ -21,13 +23,23 @@ const ConvoLink: React.FC = ({ children, }) => { return ( -
{children} @@ -42,7 +54,6 @@ const ConvoLink: React.FC = ({ e.stopPropagation(); onRename(); }} - aria-label={title || localize('com_ui_untitled')} > {title || localize('com_ui_untitled')}
= ({ aria-hidden="true" />
-
+ ); }; diff --git a/client/src/components/UnifiedSidebar/Sidebar.tsx b/client/src/components/UnifiedSidebar/Sidebar.tsx index 7c01d76ca0..642378dec9 100644 --- a/client/src/components/UnifiedSidebar/Sidebar.tsx +++ b/client/src/components/UnifiedSidebar/Sidebar.tsx @@ -7,6 +7,9 @@ import { cn } from '~/utils'; function Sidebar({ links, expanded, + width, + minWidth, + maxWidth, onCollapse, onExpand, onLeaveInsights, @@ -15,6 +18,9 @@ function Sidebar({ }: { links: NavLink[]; expanded: boolean; + width: number; + minWidth: number; + maxWidth: number; onCollapse: () => void; onExpand: () => void; onLeaveInsights: () => void; @@ -46,6 +52,9 @@ function Sidebar({ role="separator" aria-orientation="vertical" aria-label="Resize sidebar" + aria-valuenow={Math.round(width)} + aria-valuemin={Math.round(minWidth)} + aria-valuemax={Math.round(maxWidth)} tabIndex={expanded ? 0 : -1} className={cn( 'absolute right-0 top-0 z-10 h-full w-1 cursor-col-resize transition-colors hover:bg-border-medium active:bg-border-heavy', diff --git a/client/src/components/UnifiedSidebar/UnifiedSidebar.tsx b/client/src/components/UnifiedSidebar/UnifiedSidebar.tsx index 1414b6b0bb..dc0647b5e4 100644 --- a/client/src/components/UnifiedSidebar/UnifiedSidebar.tsx +++ b/client/src/components/UnifiedSidebar/UnifiedSidebar.tsx @@ -50,6 +50,7 @@ function UnifiedSidebar() { const { isSmallScreen, expanded } = useSidebarState(); const { setSidebarOpen } = useSidebarToggle(); const [sidebarWidth, setSidebarWidth] = useState(getInitialWidth); + const [viewportWidth, setViewportWidth] = useState(() => window.innerWidth); const [isResizing, setIsResizing] = useState(false); const resizeHandlers = useRef<{ move: (e: MouseEvent) => void; up: () => void } | null>(null); @@ -57,6 +58,22 @@ function UnifiedSidebar() { const isInsightsRoute = location.pathname.startsWith('/insights'); const panelExpanded = expanded && !isInsightsRoute; + /** The aside's max width is a viewport percentage, so the announced range has to track + * the viewport rather than a render-time snapshot of it. */ + useEffect(() => { + const handleViewportResize = () => setViewportWidth(window.innerWidth); + window.addEventListener('resize', handleViewportResize); + return () => window.removeEventListener('resize', handleViewportResize); + }, []); + + /** Mirrors the bounds the aside is rendered with, so the handle never announces a value + * outside its own range. CSS resolves a 40% that falls under `min-width` in favor of the + * minimum, and the resize handlers clamp the same way, so the floor belongs here too. */ + const resizeMax = Math.max(EXPANDED_MIN, Math.round(viewportWidth * 0.4)); + const resizeNow = panelExpanded + ? Math.min(Math.max(sidebarWidth, EXPANDED_MIN), resizeMax) + : COLLAPSED_WIDTH; + const handleCollapse = useCallback( (afterSlide?: () => void) => { setSidebarOpen(false, afterSlide); @@ -222,6 +239,9 @@ function UnifiedSidebar() { { + await deleteConversations(SEEDED_IDS); + await seedConversations( + getE2EUser().email, + SEEDED_IDS.map((conversationId, i) => ({ + conversationId, + title: `A11y conversation ${i + 1}`, + updatedAt: new Date(), + })), + ); +}); + +test.afterAll(async () => { + await deleteConversations(SEEDED_IDS); +}); + +/** Scanning straight after navigation catches a pre-render DOM with no main landmark and + * no composer, so waiting for the composer keeps every scan on the loaded app. Navigate + * relative to the config `baseURL` rather than a hardcoded port. */ +async function loadApp(page: Page) { + await page.goto('/', { timeout: 30000 }); + await page.getByTestId('text-input').waitFor({ state: 'visible', timeout: 30000 }); +} test('Landing page should not have any automatically detectable accessibility issues', async ({ page, }) => { - await page.goto('http://localhost:3080/', { timeout: 5000 }); + await loadApp(page); const accessibilityScanResults = await new AxeBuilder({ page }).analyze(); @@ -12,7 +45,7 @@ test('Landing page should not have any automatically detectable accessibility is }); test('Conversation page should be accessible', async ({ page }) => { - await page.goto('http://localhost:3080/', { timeout: 5000 }); + await loadApp(page); // Create a conversation (you may need to adjust this based on your app's behavior) const input = await page.locator('form').getByRole('textbox'); @@ -27,7 +60,7 @@ test('Conversation page should be accessible', async ({ page }) => { }); test('Navigation elements should be accessible', async ({ page }) => { - await page.goto('http://localhost:3080/', { timeout: 5000 }); + await loadApp(page); const navAccessibilityScanResults = await new AxeBuilder({ page }).include('nav').analyze(); @@ -35,9 +68,39 @@ test('Navigation elements should be accessible', async ({ page }) => { }); test('Input form should be accessible', async ({ page }) => { - await page.goto('http://localhost:3080/', { timeout: 5000 }); + await loadApp(page); const formAccessibilityScanResults = await new AxeBuilder({ page }).include('form').analyze(); expect(formAccessibilityScanResults.violations).toEqual([]); }); + +/** Hovering reveals the row's options button, which is what makes the row an interactive + * control containing another interactive control. Wait on that button by id rather than + * on any button in the row: the row's title control is always present, so a role match + * would be satisfied with the options control still unmounted. */ +test('Conversation list rows should be accessible with their controls revealed', async ({ + page, +}) => { + await loadApp(page); + + const row = page.getByTestId('convo-item').first(); + await expect(row).toBeVisible({ timeout: 15000 }); + await row.hover(); + await expect(row.locator('[id^="conversation-menu-"]')).toBeVisible(); + + const accessibilityScanResults = await new AxeBuilder({ page }).analyze(); + + expect(accessibilityScanResults.violations).toEqual([]); +}); + +test('Tools menu should be accessible when open', async ({ page }) => { + await loadApp(page); + + await page.locator('#tools-dropdown-button').first().click(); + await expect(page.locator('#tools-dropdown-menu')).toBeVisible({ timeout: 10000 }); + + const accessibilityScanResults = await new AxeBuilder({ page }).analyze(); + + expect(accessibilityScanResults.violations).toEqual([]); +}); diff --git a/packages/client/src/components/DropdownPopup.tsx b/packages/client/src/components/DropdownPopup.tsx index 1af4d552c0..1d1f3bfeb2 100644 --- a/packages/client/src/components/DropdownPopup.tsx +++ b/packages/client/src/components/DropdownPopup.tsx @@ -19,7 +19,7 @@ interface DropdownProps { gutter?: number; modal?: boolean; portal?: boolean; - portalElement?: HTMLElement | null; + portalElement?: Ariakit.MenuProps['portalElement']; preserveTabOrder?: boolean; focusLoop?: boolean; menuId: string;