Skip to content

Commit

Permalink
fix: Fix accessibility findings with app layout toolbar (#3063)
Browse files Browse the repository at this point in the history
  • Loading branch information
avinashbot authored Dec 4, 2024
1 parent 7948389 commit a5be1f7
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 17 deletions.
6 changes: 5 additions & 1 deletion pages/app-layout/disable-paddings-breadcrumbs.page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ export default function () {
<AppLayout
ariaLabels={labels}
disableContentPaddings={true}
breadcrumbs={<div className={styles.highlightBorder}>Breadcrumbs</div>}
breadcrumbs={
<nav aria-label="Breadcrumbs" className={styles.highlightBorder}>
Breadcrumbs
</nav>
}
notifications={
<div className={styles.highlightBorder}>
<Box variant="h2">Notifications</Box>
Expand Down
3 changes: 2 additions & 1 deletion pages/app-layout/runtime-drawers.page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import awsuiPlugins from '~components/internal/plugins';
import './utils/external-widget';
import AppContext, { AppContextType } from '../app/app-context';
import { Breadcrumbs, Containers, CustomDrawerContent } from './utils/content-blocks';
import { drawerLabels } from './utils/drawers';
import appLayoutLabels from './utils/labels';

type DemoContext = React.Context<
Expand Down Expand Up @@ -64,7 +65,7 @@ export default function WithDrawers() {

return (
<AppLayout
ariaLabels={appLayoutLabels}
ariaLabels={{ ...appLayoutLabels, ...drawerLabels }}
breadcrumbs={<Breadcrumbs />}
ref={appLayoutRef}
content={
Expand Down
6 changes: 5 additions & 1 deletion pages/app-layout/stateful.page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ export default function AppLayoutStatefulDemo() {
return (
<AppLayout
ariaLabels={labels}
breadcrumbs={<Counter id="breadcrumbs" />}
breadcrumbs={
<nav aria-label="Breadcrumbs">
<Counter id="breadcrumbs" />
</nav>
}
navigation={<Counter id="navigation" />}
tools={<Counter id="tools" />}
content={
Expand Down
9 changes: 7 additions & 2 deletions pages/app-layout/utils/content-blocks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,15 @@ export function CustomDrawerContent() {
<div className={styles['custom-drawer-wrapper']}>
<div className={styles['drawer-sticky-header']} data-testid="drawer-sticky-header">
<Box variant="h3" tagOverride="h2" padding="n">
Sticky header
<span id="custom-drawer-heading">Sticky header</span>
</Box>
</div>
<div className={styles['drawer-scrollable-content']}>
<div
className={styles['drawer-scrollable-content']}
role="region"
aria-labelledby="custom-drawer-heading"
tabIndex={0}
>
<ScrollableDrawerContent />
</div>
<div className={styles['drawer-sticky-footer']} data-testid="drawer-sticky-footer">
Expand Down
30 changes: 30 additions & 0 deletions src/__a11y__/a11y-app-layout-toolbar.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import useBrowser from '@cloudscape-design/browser-test-tools/use-browser';

import { findAllPages } from '../__integ__/utils';
import { getUrlParams } from '../app-layout/__integ__/utils';
import A11yPageObject from './a11y-page-object';

const EXCLUDED_PAGES = [
// Test page for an app layout nested inside another through an iframe.
// Not a use case that's encouraged.
'app-layout/multi-layout-global-drawer-child-layout',
];

describe('A11y checks for app layout toolbar', () => {
findAllPages()
.filter(page => page.startsWith('app-layout') && !EXCLUDED_PAGES.includes(page))
.forEach(inputUrl => {
const url = `#/light/${inputUrl}?${getUrlParams('refresh-toolbar')}`;
test(
url,
useBrowser(async browser => {
const page = new A11yPageObject(browser);
await browser.url(url);
await page.waitForVisible('main');
await page.assertNoAxeViolations();
})
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export function AppLayoutDrawerImplementation({ appLayoutInternals }: AppLayoutD
[customCssProps.drawerSize]: `${['entering', 'entered'].includes(state) ? size : 0}px`,
}),
}}
data-testid={`awsui-app-layout-drawer-${activeDrawerId}`}
data-testid={activeDrawerId && `awsui-app-layout-drawer-${activeDrawerId}`}
>
{!isMobile && activeDrawer?.resizable && (
<div className={styles['drawer-slider']}>
Expand Down
5 changes: 3 additions & 2 deletions src/app-layout/visual-refresh-toolbar/split-panel/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,10 @@ export function AppLayoutSplitPanelDrawerBottomImplementation({
splitPanelInternals,
appLayoutInternals,
}: AppLayoutSplitPanelDrawerBottomImplementationProps) {
const { splitPanelControlId, splitPanelAnimationDisabled } = appLayoutInternals;
return (
<SplitPanelProvider {...splitPanelInternals} animationDisabled={appLayoutInternals.splitPanelAnimationDisabled}>
{children}
<SplitPanelProvider {...splitPanelInternals} animationDisabled={splitPanelAnimationDisabled}>
<section id={splitPanelControlId}>{children}</section>
</SplitPanelProvider>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,7 @@ export function DrawerTriggers({
ref={triggersContainerRef}
role="region"
>
<div
className={clsx(styles['drawers-trigger-content'], {
[styles['has-multiple-triggers']]: hasMultipleTriggers,
[styles['has-open-drawer']]: activeDrawerId,
})}
role="toolbar"
aria-orientation="horizontal"
>
<div className={styles['drawers-trigger-content']} role="toolbar" aria-orientation="horizontal">
{splitPanelToggleProps && (
<>
<TriggerButton
Expand Down
5 changes: 4 additions & 1 deletion src/app-layout/visual-refresh-toolbar/toolbar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,9 @@ export function AppLayoutToolbarImplementation({
}, [anyPanelOpenInMobile]);

const toolbarHidden = toolbarState === 'hide' && !pinnedToolbar;
const navLandmarkAttributes = navigationOpen
? { role: 'presentation' }
: { role: 'navigation', 'aria-label': ariaLabels?.navigation };

return (
<ToolbarSlot
Expand All @@ -180,7 +183,7 @@ export function AppLayoutToolbarImplementation({
>
<div className={styles['toolbar-container']}>
{hasNavigation && (
<nav className={clsx(styles['universal-toolbar-nav'])}>
<nav {...navLandmarkAttributes} className={clsx(styles['universal-toolbar-nav'])}>
<TriggerButton
ariaLabel={ariaLabels?.navigationToggle ?? undefined}
ariaExpanded={false}
Expand Down

0 comments on commit a5be1f7

Please sign in to comment.