Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Provide accessible name to menu in button-dropdown #1403

Merged
merged 5 commits into from
Aug 24, 2023
Merged

feat: Provide accessible name to menu in button-dropdown #1403

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1e6dec5
feat: Provide accessible name to menu in button-dropdown
Al-Dani Aug 2, 2023
cab6f7b
test: Add unit tests
Al-Dani Aug 2, 2023
50d9bdd
test: Add more tests
Al-Dani Aug 9, 2023
f01e669
fix: Exclude main action variant
Al-Dani Aug 24, 2023
5141cc6
test: Add tests for a button with main action
Al-Dani Aug 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions src/button-dropdown/__tests__/button-dropdown-accessibility.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ const renderButtonDropdown = (props: ButtonDropdownProps) => {
return createWrapper(renderResult.container).findButtonDropdown()!;
};

const renderWithTrigger = (props: ButtonDropdownProps, triggerName: string) => {
const renderResult = render(<ButtonDropdown {...props}>{triggerName}</ButtonDropdown>);
return createWrapper(renderResult.container).findButtonDropdown()!;
};

const items: ButtonDropdownProps.Items = [
{ id: 'i1', text: 'item1', description: 'Item 1 description' },
{
Expand Down Expand Up @@ -67,6 +72,68 @@ const items: ButtonDropdownProps.Items = [
expect(groups[0].findAll('[role="menuitem"]').length).toBe(2);
expect(groups[1].findAll('[role="menuitem"]').length).toBe(1);
});

it('auto-labels dropdown with trigger title', () => {
const wrapper = renderWithTrigger({ ...props }, 'Actions');
wrapper.openDropdown();

const menuElement = wrapper.findOpenDropdown()!.find('[role="menu"]')!;
expect(menuElement.getElement()).toHaveAccessibleName('Actions');
});

it('uses aria-label as accessible name for dropdown if presented', () => {
const wrapper = renderWithTrigger({ ...props, ariaLabel: 'Custom label' }, 'Actions');
wrapper.openDropdown();

const menuElement = wrapper.findOpenDropdown()!.find('[role="menu"]')!;
expect(menuElement.getElement()).toHaveAccessibleName('Custom label');
});

it('does not auto-label dropdown with icon trigger', () => {
const wrapper = renderButtonDropdown({ ...props, variant: 'icon' });
wrapper.openDropdown();
const menuElement = wrapper.findOpenDropdown()!.find('[role="menu"]')!;
expect(menuElement.getElement()).not.toHaveAttribute('aria-labelledby');
});
it('passes aria-label to icon trigger', () => {
const wrapper = renderButtonDropdown({ ...props, variant: 'icon', ariaLabel: 'Button trigger' });
wrapper.openDropdown();
const menuElement = wrapper.findOpenDropdown()!.find('[role="menu"]')!;
expect(menuElement.getElement()).toHaveAccessibleName('Button trigger');
});

it('does not auto-label dropdown with inline icon trigger', () => {
const wrapper = renderButtonDropdown({ ...props, variant: 'inline-icon' });
wrapper.openDropdown();
const menuElement = wrapper.findOpenDropdown()!.find('[role="menu"]')!;
expect(menuElement.getElement()).not.toHaveAttribute('aria-labelledby');
});
it('passes aria-label to inline icon trigger', () => {
const wrapper = renderButtonDropdown({ ...props, variant: 'inline-icon', ariaLabel: 'Button trigger' });
wrapper.openDropdown();
const menuElement = wrapper.findOpenDropdown()!.find('[role="menu"]')!;
expect(menuElement.getElement()).toHaveAccessibleName('Button trigger');
});

it('uses aria-label to label dropdown menu in a button with main action', () => {
const wrapper = renderButtonDropdown({
...props,
mainAction: { text: 'Main action', ariaLabel: 'Main action aria label' },
ariaLabel: 'Actions',
});
wrapper.openDropdown();
const menuElement = wrapper.findOpenDropdown()!.find('[role="menu"]')!;
expect(menuElement.getElement()).toHaveAccessibleName('Actions');
});
it('does not auto-label dropdown in a button with main action', () => {
const wrapper = renderButtonDropdown({
...props,
mainAction: { text: 'Main action', ariaLabel: 'Main action aria label' },
});
wrapper.openDropdown();
const menuElement = wrapper.findOpenDropdown()!.find('[role="menu"]')!;
expect(menuElement.getElement()).not.toHaveAccessibleName();
});
});
});

Expand Down
9 changes: 7 additions & 2 deletions src/button-dropdown/internal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ const InternalButtonDropdown = React.forwardRef(
},
};

const triggerId = useUniqueId('awsui-button-dropdown__trigger');

const triggerHasBadge = () => {
const flatItems = items.flatMap(item => {
if ('items' in item) {
Expand Down Expand Up @@ -215,7 +217,7 @@ const InternalButtonDropdown = React.forwardRef(
} else {
trigger = (
<div className={styles['dropdown-trigger']}>
<InternalButton ref={triggerRef} {...baseTriggerProps} badge={triggerHasBadge()}>
<InternalButton ref={triggerRef} id={triggerId} {...baseTriggerProps} badge={triggerHasBadge()}>
{children}
</InternalButton>
</div>
Expand All @@ -225,6 +227,8 @@ const InternalButtonDropdown = React.forwardRef(
const hasHeader = title || description;
const headerId = useUniqueId('awsui-button-dropdown__header');

const shouldLabelWithTrigger = !ariaLabel && !mainAction && variant !== 'icon' && variant !== 'inline-icon';

const { loadingButtonCount } = useFunnel();
useEffect(() => {
if (loading) {
Expand Down Expand Up @@ -284,7 +288,8 @@ const InternalButtonDropdown = React.forwardRef(
position="static"
role="menu"
decreaseTopMargin={true}
ariaLabelledby={hasHeader ? headerId : undefined}
ariaLabel={ariaLabel}
ariaLabelledby={hasHeader ? headerId : shouldLabelWithTrigger ? triggerId : undefined}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is header and why trigger ARIA label is preferred over the header?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

header is a combination of title and description, these properties are available only internally and can be used with variant: 'navigation' inside TopNavigation.

Priorities are the following:

  • if there is a header inside dropdown, than it is used to label menu list
  • else if we have aria-label - use it
  • else if the variant is not icon or inline-icon - label by trigger text

In case of a split button variant I assume there will be an aria-label, because trigger itself is an icon and we can't auto-label dropdown with it

statusType="finished"
>
<ItemsList
Expand Down
3 changes: 3 additions & 0 deletions src/internal/components/options-list/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export interface OptionsListProps extends BaseComponentProps {
onMouseMove?: (itemIndex: number) => void;
position?: React.CSSProperties['position'];
role?: 'listbox' | 'list' | 'menu';
ariaLabel?: string;
ariaLabelledby?: string;
ariaDescribedby?: string;
decreaseTopMargin?: boolean;
Expand Down Expand Up @@ -66,6 +67,7 @@ const OptionsList = (
position = 'relative',
role = 'listbox',
decreaseTopMargin = false,
ariaLabel,
ariaLabelledby,
ariaDescribedby,
...restProps
Expand Down Expand Up @@ -113,6 +115,7 @@ const OptionsList = (
onBlur={event => fireNonCancelableEvent(onBlur, { relatedTarget: event.relatedTarget })}
onFocus={() => fireNonCancelableEvent(onFocus)}
tabIndex={-1}
aria-label={ariaLabel}
aria-labelledby={ariaLabelledby}
aria-describedby={ariaDescribedby}
>
Expand Down
Loading