mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-27 04:07:05 +00:00
♿ fix: Resolve axe Violations in Sidebar, Tools Dropdown and Footer (#14979)
* ♿ 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
This commit is contained in:
parent
c5276fc63d
commit
e49e264487
9 changed files with 126 additions and 36 deletions
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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<HTMLElement>('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}
|
||||
|
|
|
|||
|
|
@ -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<MeasuredRowProps> = memo(
|
||||
({ cache, rowKey, parent, index, style, children }) => (
|
||||
<CellMeasurer cache={cache} columnIndex={0} key={rowKey} parent={parent} rowIndex={index}>
|
||||
|
|
@ -63,8 +65,9 @@ const MeasuredRow: FC<MeasuredRowProps> = memo(
|
|||
style={style}
|
||||
className="px-3"
|
||||
data-testid="convo-list-row"
|
||||
role="row"
|
||||
>
|
||||
{children}
|
||||
<div role="gridcell">{children}</div>
|
||||
</div>
|
||||
)}
|
||||
</CellMeasurer>
|
||||
|
|
|
|||
|
|
@ -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({
|
|||
<ConvoLink
|
||||
isActiveConvo={isActiveConvo}
|
||||
isPopoverActive={isPopoverActive}
|
||||
isSharedBadgeVisible={isSharedBadgeVisible}
|
||||
title={title}
|
||||
onRename={handleRename}
|
||||
isSmallScreen={isSmallScreen}
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import { cn } from '~/utils';
|
|||
interface ConvoLinkProps {
|
||||
isActiveConvo: boolean;
|
||||
isPopoverActive: boolean;
|
||||
isSharedBadgeVisible: boolean;
|
||||
title: string | null;
|
||||
onRename: () => void;
|
||||
isSmallScreen: boolean;
|
||||
|
|
@ -14,6 +15,7 @@ interface ConvoLinkProps {
|
|||
const ConvoLink: React.FC<ConvoLinkProps> = ({
|
||||
isActiveConvo,
|
||||
isPopoverActive,
|
||||
isSharedBadgeVisible,
|
||||
title,
|
||||
onRename,
|
||||
isSmallScreen,
|
||||
|
|
@ -21,13 +23,23 @@ const ConvoLink: React.FC<ConvoLinkProps> = ({
|
|||
children,
|
||||
}) => {
|
||||
return (
|
||||
<div
|
||||
<button
|
||||
type="button"
|
||||
className={cn(
|
||||
'flex min-w-0 grow items-center gap-2 overflow-hidden rounded-lg px-2',
|
||||
'flex min-w-0 grow cursor-pointer items-center gap-2 overflow-hidden rounded-lg px-2 text-left outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-text-primary',
|
||||
isActiveConvo || isPopoverActive ? 'bg-surface-active-alt' : '',
|
||||
)}
|
||||
title={title ?? undefined}
|
||||
aria-current={isActiveConvo ? 'page' : undefined}
|
||||
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'),
|
||||
})
|
||||
}
|
||||
style={{ width: '100%' }}
|
||||
>
|
||||
{children}
|
||||
|
|
@ -42,7 +54,6 @@ const ConvoLink: React.FC<ConvoLinkProps> = ({
|
|||
e.stopPropagation();
|
||||
onRename();
|
||||
}}
|
||||
aria-label={title || localize('com_ui_untitled')}
|
||||
>
|
||||
{title || localize('com_ui_untitled')}
|
||||
<div
|
||||
|
|
@ -55,7 +66,7 @@ const ConvoLink: React.FC<ConvoLinkProps> = ({
|
|||
aria-hidden="true"
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
</button>
|
||||
);
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -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() {
|
|||
<Sidebar
|
||||
links={links}
|
||||
expanded={panelExpanded}
|
||||
width={resizeNow}
|
||||
minWidth={panelExpanded ? EXPANDED_MIN : COLLAPSED_WIDTH}
|
||||
maxWidth={panelExpanded ? resizeMax : COLLAPSED_WIDTH}
|
||||
onCollapse={handleCollapse}
|
||||
onExpand={handlePanelExpand}
|
||||
onLeaveInsights={handleLeaveInsights}
|
||||
|
|
|
|||
|
|
@ -1,10 +1,43 @@
|
|||
import { expect, test } from '@playwright/test';
|
||||
import type { Page } from '@playwright/test';
|
||||
import AxeBuilder from '@axe-core/playwright'; // 1
|
||||
import { deleteConversations, seedConversations } from './mock/db';
|
||||
import { getE2EUser } from '../setup/user';
|
||||
|
||||
const SEEDED_IDS = ['a11y-spec-convo-1', 'a11y-spec-convo-2'];
|
||||
|
||||
/** A fresh e2e user has no conversations, so without seeding the sidebar renders no rows
|
||||
* and no scan below reaches the conversation row markup. The pre-delete keeps the suite
|
||||
* idempotent: the ids are fixed and conversations are uniquely indexed, so a run that
|
||||
* dies before afterAll would otherwise leave the next one to fail on insert. */
|
||||
test.beforeAll(async () => {
|
||||
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([]);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue