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.

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

fix(toolbar): missing aria orientation attribute #669

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/components/AriaToolbar/AriaToolbar.constants.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const CLASS_PREFIX = 'md-aria-toolbar';

const DEFAULTS = {
ORIENTATION: 'horizontal',
ORIENTATION: 'horizontal' as const,
SHOULD_RENDER_AS_BUTTON_GROUP: false,
};

Expand Down
5 changes: 3 additions & 2 deletions src/components/AriaToolbar/AriaToolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const AriaToolbar: FC<Props> = (props: Props) => {
id,
style,
children,
orientation = DEFAULTS.ORIENTATION as Props['orientation'],
orientation = DEFAULTS.ORIENTATION,
shouldRenderAsButtonGroup = DEFAULTS.SHOULD_RENDER_AS_BUTTON_GROUP,
onTabPress,
ariaControls,
Expand Down Expand Up @@ -55,6 +55,7 @@ const AriaToolbar: FC<Props> = (props: Props) => {
'aria-label': ariaLabel,
'aria-controls': ariaControls,
role: 'toolbar',
'aria-orientation': orientation,
...rest,
};

Expand All @@ -76,7 +77,7 @@ const AriaToolbar: FC<Props> = (props: Props) => {

useEffect(() => {
// When the toolbar is rendered inside a list, only the first item in the toolbar
// should be focusable. This is the preserve the tab order as the
// should be focusable. This is to preserve the tab order as the
// List uses a roving tab index.
getKeyboardFocusableElements(ref.current, false).forEach((el, index) => {
if (index === 0) {
Expand Down
25 changes: 25 additions & 0 deletions src/components/AriaToolbar/AriaToolbar.unit.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,31 @@ describe('<AriaToolbar />', () => {

expect(element.getAttribute('aria-label')).toBe(ariaLabel);
});

it.each`
orientation | ariaOrientation
${'horizontal'} | ${'horizontal'}
${'vertical'} | ${'vertical'}
${undefined} | ${'horizontal'}
`(
'should have provided aria-orientation when orientation is $orientation',
({ orientation, ariaOrientation }) => {
expect.assertions(1);

const element = mount(
<AriaToolbar
shouldRenderAsButtonGroup
ariaLabel="test"
orientation={orientation}
ariaToolbarItemsSize={0}
/>
)
.find(AriaToolbar)
.getDOMNode();

expect(element.getAttribute('aria-orientation')).toBe(ariaOrientation);
}
);
});

describe('actions', () => {
Expand Down
19 changes: 19 additions & 0 deletions src/components/AriaToolbar/AriaToolbar.unit.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ exports[`<AriaToolbar /> snapshot should match snapshot 1`] = `
>
<div
aria-label="test"
aria-orientation="horizontal"
className="md-aria-toolbar-wrapper"
role="toolbar"
/>
Expand All @@ -26,6 +27,7 @@ exports[`<AriaToolbar /> snapshot should match snapshot with aria label + button
>
<ButtonGroup
aria-label="test"
aria-orientation="horizontal"
className="md-aria-toolbar-wrapper"
orientation="horizontal"
role="toolbar"
Expand All @@ -37,6 +39,7 @@ exports[`<AriaToolbar /> snapshot should match snapshot with aria label + button
>
<div
aria-label="test"
aria-orientation="horizontal"
className="md-button-group-wrapper md-aria-toolbar-wrapper"
data-compressed={false}
data-orientation="horizontal"
Expand All @@ -62,6 +65,7 @@ exports[`<AriaToolbar /> snapshot should match snapshot with className 1`] = `
>
<div
aria-label="test"
aria-orientation="horizontal"
className="example-class md-aria-toolbar-wrapper"
role="toolbar"
/>
Expand All @@ -76,6 +80,7 @@ exports[`<AriaToolbar /> snapshot should match snapshot with data-test 1`] = `
>
<div
aria-label="test"
aria-orientation="horizontal"
className="md-aria-toolbar-wrapper"
data-test="data-test"
role="toolbar"
Expand All @@ -91,6 +96,7 @@ exports[`<AriaToolbar /> snapshot should match snapshot with id 1`] = `
>
<div
aria-label="test"
aria-orientation="horizontal"
className="md-aria-toolbar-wrapper"
id="example-id"
role="toolbar"
Expand All @@ -110,6 +116,7 @@ exports[`<AriaToolbar /> snapshot should match snapshot with style 1`] = `
>
<div
aria-label="test"
aria-orientation="horizontal"
className="md-aria-toolbar-wrapper"
role="toolbar"
style={
Expand All @@ -128,6 +135,7 @@ exports[`<AriaToolbar /> snapshot should set tab index appropriately - horizonta
>
<div
aria-label="test"
aria-orientation="horizontal"
className="md-aria-toolbar-wrapper"
role="toolbar"
>
Expand Down Expand Up @@ -255,6 +263,7 @@ exports[`<AriaToolbar /> snapshot should set tab index appropriately - horizonta
>
<div
aria-label="test"
aria-orientation="horizontal"
className="md-aria-toolbar-wrapper"
role="toolbar"
>
Expand Down Expand Up @@ -382,6 +391,7 @@ exports[`<AriaToolbar /> snapshot should set tab index appropriately - horizonta
>
<div
aria-label="test"
aria-orientation="horizontal"
className="md-aria-toolbar-wrapper"
role="toolbar"
>
Expand Down Expand Up @@ -509,6 +519,7 @@ exports[`<AriaToolbar /> snapshot should set tab index appropriately - horizonta
>
<div
aria-label="test"
aria-orientation="horizontal"
className="md-aria-toolbar-wrapper"
role="toolbar"
>
Expand Down Expand Up @@ -637,6 +648,7 @@ exports[`<AriaToolbar /> snapshot should set tab index appropriately - vertical
>
<div
aria-label="test"
aria-orientation="vertical"
className="md-aria-toolbar-wrapper"
role="toolbar"
>
Expand Down Expand Up @@ -765,6 +777,7 @@ exports[`<AriaToolbar /> snapshot should set tab index appropriately - vertical
>
<div
aria-label="test"
aria-orientation="vertical"
className="md-aria-toolbar-wrapper"
role="toolbar"
>
Expand Down Expand Up @@ -893,6 +906,7 @@ exports[`<AriaToolbar /> snapshot should set tab index appropriately - vertical
>
<div
aria-label="test"
aria-orientation="vertical"
className="md-aria-toolbar-wrapper"
role="toolbar"
>
Expand Down Expand Up @@ -1021,6 +1035,7 @@ exports[`<AriaToolbar /> snapshot should set tab index appropriately - vertical
>
<div
aria-label="test"
aria-orientation="vertical"
className="md-aria-toolbar-wrapper"
role="toolbar"
>
Expand Down Expand Up @@ -1145,6 +1160,7 @@ exports[`<AriaToolbar /> snapshot should update order if triggered externally 1`
<div>
<div
aria-label="test"
aria-orientation="vertical"
class="md-aria-toolbar-wrapper"
role="toolbar"
>
Expand Down Expand Up @@ -1172,6 +1188,7 @@ exports[`<AriaToolbar /> snapshot should update order if triggered externally 2`
<div>
<div
aria-label="test"
aria-orientation="vertical"
class="md-aria-toolbar-wrapper"
role="toolbar"
>
Expand Down Expand Up @@ -1199,6 +1216,7 @@ exports[`<AriaToolbar /> snapshot should update order if triggered externally 3`
<div>
<div
aria-label="test"
aria-orientation="vertical"
class="md-aria-toolbar-wrapper"
role="toolbar"
>
Expand Down Expand Up @@ -1226,6 +1244,7 @@ exports[`<AriaToolbar /> snapshot should update order if triggered externally 4`
<div>
<div
aria-label="test"
aria-orientation="vertical"
class="md-aria-toolbar-wrapper"
role="toolbar"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ exports[`<ReactionPicker /> snapshot should match snapshot 1`] = `
>
<div
aria-label="test-aria-label"
aria-orientation="horizontal"
className="md-reaction-picker-wrapper md-aria-toolbar-wrapper"
role="toolbar"
/>
Expand All @@ -33,6 +34,7 @@ exports[`<ReactionPicker /> snapshot should match snapshot with children 1`] = `
>
<div
aria-label="test-aria-label"
aria-orientation="horizontal"
className="md-reaction-picker-wrapper md-aria-toolbar-wrapper"
role="toolbar"
>
Expand Down Expand Up @@ -132,6 +134,7 @@ exports[`<ReactionPicker /> snapshot should match snapshot with className 1`] =
>
<div
aria-label="test-aria-label"
aria-orientation="horizontal"
className="example-class md-reaction-picker-wrapper md-aria-toolbar-wrapper"
role="toolbar"
/>
Expand All @@ -154,6 +157,7 @@ exports[`<ReactionPicker /> snapshot should match snapshot with id 1`] = `
>
<div
aria-label="test-aria-label"
aria-orientation="horizontal"
className="md-reaction-picker-wrapper md-aria-toolbar-wrapper"
id="example-id"
role="toolbar"
Expand Down Expand Up @@ -185,6 +189,7 @@ exports[`<ReactionPicker /> snapshot should match snapshot with style 1`] = `
>
<div
aria-label="test-aria-label"
aria-orientation="horizontal"
className="md-reaction-picker-wrapper md-aria-toolbar-wrapper"
role="toolbar"
style={
Expand Down
Loading