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(Tabs): improve screen reader support #6989

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
92f98a0
fix(Tab): update role
emyarod Oct 7, 2020
26ea3cd
fix(Tab): place aria attributes on inner anchor instead of li
emyarod Oct 7, 2020
aa39bc5
test(Tabs): update tab role test
emyarod Oct 7, 2020
157d4d8
fix(Tab): convert anchor to button
emyarod Oct 7, 2020
8b05c26
test(Tabs): update inner anchor tests
emyarod Oct 7, 2020
19a6ab7
chore: update snapshots
emyarod Oct 7, 2020
ac42045
test(Tab): update test descriptions
emyarod Oct 12, 2020
bcb3ad1
test(Tab): remove obsolete test
emyarod Oct 12, 2020
3eda856
fix(Tab): remove tabIndex from wrapper li
emyarod Oct 12, 2020
8972b19
fix(Tab): add role to button
emyarod Oct 12, 2020
0ca0b97
fix(Tab): deprecate required href prop
emyarod Oct 12, 2020
10e0879
fix(TabContent): remove `aria-live` and `aria-hidden`
emyarod Oct 12, 2020
90af573
chore: update snapshots
emyarod Oct 12, 2020
75b2917
test(Tab): update test description
emyarod Oct 12, 2020
c7e7bcf
chore(Tab): deprecate tabIndex prop
emyarod Oct 14, 2020
0a550fe
docs(Tabs): add tabpanel role to custom tab content wrapper
emyarod Oct 14, 2020
1c52293
chore: update snapshots
emyarod Oct 14, 2020
9f1bf91
fix(Tabs): activate scroll buttons with LMB only
emyarod Oct 16, 2020
6de3873
fix(Tabs): prevent keyboard focus of scroll buttons
emyarod Oct 16, 2020
a13f328
fix(Tabs): prevent ul from being focusable
emyarod Oct 21, 2020
1b2a245
refactor(Tabs): carry over changes from #7045
emyarod Oct 23, 2020
30bead8
Merge branch 'master' into 6984-tabs-control-screen-reader-support
kodiakhq[bot] Oct 23, 2020
bb28519
Merge branch 'master' into 6984-tabs-control-screen-reader-support
kodiakhq[bot] Oct 23, 2020
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
3 changes: 3 additions & 0 deletions packages/components/src/components/tabs/_tabs.scss
Original file line number Diff line number Diff line change
Expand Up @@ -689,13 +689,16 @@
// Link
//-----------------------------
.#{$prefix}--tabs--scrollable__nav-link {
@include button-reset($width: false);
@include focus-outline('reset');
@include type-style('body-short-01');

width: rem(160px);
padding: $spacing-04 $spacing-05 $spacing-03;
overflow: hidden;
color: $text-02;
white-space: nowrap;
text-align: left;
text-decoration: none;
text-overflow: ellipsis;
border-bottom: $tab-underline-color;
Expand Down
14 changes: 3 additions & 11 deletions packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -4953,10 +4953,7 @@ Map {
"handleTabKeyDown": Object {
"type": "func",
},
"href": Object {
"isRequired": true,
"type": "string",
},
"href": [Function],
"id": Object {
"type": "string",
},
Expand All @@ -4974,9 +4971,7 @@ Map {
"isRequired": true,
"type": "func",
},
"renderAnchor": Object {
"type": "func",
},
"renderAnchor": [Function],
"renderContent": Object {
"type": "func",
},
Expand All @@ -4988,10 +4983,7 @@ Map {
"isRequired": true,
"type": "bool",
},
"tabIndex": Object {
"isRequired": true,
"type": "number",
},
"tabIndex": [Function],
},
},
"TabContent" => Object {
Expand Down
31 changes: 11 additions & 20 deletions packages/react/src/components/Tab/Tab-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,38 +21,29 @@ describe('Tab', () => {
expect(wrapper.hasClass('extra-class')).toBe(true);
});

it('renders <a> with expected className', () => {
it('renders <button> with expected className', () => {
expect(
// TODO: uncomment and replace assertion in next major version
// wrapper.find('a').hasClass(`${prefix}--tabs__nav-link`)
wrapper.find('a').hasClass(`${prefix}--tabs--scrollable__nav-link`)
// wrapper.find('button').hasClass(`${prefix}--tabs__nav-link`)
wrapper.find('button').hasClass(`${prefix}--tabs--scrollable__nav-link`)
).toBe(true);
});

it('renders <li> with [role="tab"]', () => {
expect(wrapper.props().role).toEqual('tab');
it('renders <li> with [role="presentation"]', () => {
expect(wrapper.props().role).toEqual('presentation');
});

it('renders <a> with tabindex set to 0', () => {
expect(wrapper.find('a').props().tabIndex).toEqual(0);
it('renders <button> with tabindex set to 0', () => {
expect(wrapper.find('button').props().tabIndex).toEqual(0);
});

it('sets tabIndex on <a> if one is passed via props', () => {
it('sets tabIndex on <button> if one is passed via props', () => {
wrapper.setProps({ tabIndex: 2 });
expect(wrapper.find('a').props().tabIndex).toEqual(2);
expect(wrapper.find('button').props().tabIndex).toEqual(2);
});

it('uses label to set children on <a> when passed via props', () => {
expect(wrapper.find('a').props().children).toEqual('firstTab');
});

it('sets href as # by default', () => {
expect(wrapper.find('a').props().href).toEqual('#');
});

it('sets new href value when passed in via props', () => {
wrapper.setProps({ href: '#other-content' });
expect(wrapper.find('a').props().href).toEqual('#other-content');
it('uses label to set children on <button> when passed via props', () => {
expect(wrapper.find('button').props().children).toEqual('firstTab');
});

it(`should not have [className="${prefix}--tabs__nav-item--selected"] by default`, () => {
Expand Down
31 changes: 17 additions & 14 deletions packages/react/src/components/Tab/Tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import React from 'react';
import classNames from 'classnames';
import { settings } from 'carbon-components';
import TabContent from '../TabContent';
import deprecate from '../../prop-types/deprecate.js';

const { prefix } = settings;

Expand Down Expand Up @@ -40,7 +41,7 @@ export default class Tab extends React.Component {
/**
* Provide a string that represents the `href` of the Tab
*/
href: PropTypes.string.isRequired,
href: deprecate(PropTypes.string),

/**
* The element ID for the top-level element.
Expand Down Expand Up @@ -72,7 +73,7 @@ export default class Tab extends React.Component {
* Useful for using Tab along with react-router or other client
* side router libraries.
**/
renderAnchor: PropTypes.func,
renderAnchor: deprecate(PropTypes.func),

/*
* An optional parameter to allow overriding the content rendering.
Expand All @@ -91,9 +92,9 @@ export default class Tab extends React.Component {
selected: PropTypes.bool.isRequired,

/**
* Specify the tab index of the `<a>` node
* Specify the tab index of the `<button>` node
*/
tabIndex: PropTypes.number.isRequired,
tabIndex: deprecate(PropTypes.number),
};

static defaultProps = {
Expand Down Expand Up @@ -121,7 +122,8 @@ export default class Tab extends React.Component {
tabIndex,
emyarod marked this conversation as resolved.
Show resolved Hide resolved
onClick,
onKeyDown,
renderAnchor,
// TODO: rename renderAnchor to renderButton in next major version
renderAnchor: renderButton,
renderContent, // eslint-disable-line no-unused-vars
...other
} = this.props;
Expand All @@ -140,7 +142,10 @@ export default class Tab extends React.Component {
}
);

const anchorProps = {
const buttonProps = {
['aria-selected']: selected,
['aria-disabled']: disabled,
['aria-controls']: `${id}__panel`,
id,
// TODO: remove scrollable in next major release
// className: `${prefix}--tabs__nav-link`,
Expand All @@ -155,7 +160,6 @@ export default class Tab extends React.Component {
return (
<li
{...other}
tabIndex={-1}
className={classes}
onClick={(evt) => {
if (disabled) {
Expand All @@ -171,14 +175,13 @@ export default class Tab extends React.Component {
handleTabKeyDown(index, evt);
onKeyDown(evt);
}}
role="tab"
aria-selected={selected}
aria-disabled={disabled}
aria-controls={`${id}__panel`}>
{renderAnchor ? (
renderAnchor(anchorProps)
role="presentation">
{renderButton ? (
renderButton(buttonProps)
) : (
<a {...anchorProps}>{label}</a>
<button type="button" role="tab" {...buttonProps}>
{label}
</button>
)}
</li>
);
Expand Down
3 changes: 1 addition & 2 deletions packages/react/src/components/TabContent/TabContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ const TabContent = (props) => {
{...other}
className={tabContentClasses}
selected={selected}
hidden={!selected}
aria-live="polite">
hidden={!selected}>
{children}
</div>
);
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/components/Tabs/Tabs-story.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ const props = {
}),
tab: () => ({
disabled: boolean('Disabled (disabled in <Tab>)', false),
href: text('The href for tab (href in <Tab>)', '#'),
tabIndex: number('Tab index (tabIndex in <Tab>)', 0),
onClick: action('onClick'),
onKeyDown: action('onKeyDown'),
Expand Down Expand Up @@ -96,6 +95,7 @@ const TabContentRenderedOnlyWhenSelected = ({
<div
{...other}
className={classNames(className, `${prefix}--tab-content`)}
role="tabpanel"
selected={selected}>
{children}
</div>
Expand Down
60 changes: 30 additions & 30 deletions packages/react/src/components/Tabs/Tabs-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,10 @@ describe('Tabs', () => {
let wrapper;
let firstTab;
let lastTab;
let anchorInFirstTab;
let anchorInLastTab;
let spyFocusAnchorInFirstTab;
let spyFocusAnchorInLastTab;
let buttonInFirstTab;
let buttonInLastTab;
let spyFocusButtonInFirstTab;
let spyFocusButtonInLastTab;

describe('state: selected', () => {
beforeEach(() => {
Expand All @@ -163,29 +163,29 @@ describe('Tabs', () => {
);
firstTab = wrapper.find('.firstTab').last();
lastTab = wrapper.find('.lastTab').last();
anchorInFirstTab = firstTab.find('a').getDOMNode();
anchorInLastTab = lastTab.find('a').getDOMNode();
buttonInFirstTab = firstTab.find('button').getDOMNode();
buttonInLastTab = lastTab.find('button').getDOMNode();
});

it('updates selected state when pressing arrow keys', () => {
spyFocusAnchorInFirstTab = jest.spyOn(anchorInFirstTab, 'focus');
spyFocusAnchorInLastTab = jest.spyOn(anchorInLastTab, 'focus');
spyFocusButtonInFirstTab = jest.spyOn(buttonInFirstTab, 'focus');
spyFocusButtonInLastTab = jest.spyOn(buttonInLastTab, 'focus');
firstTab.simulate('keydown', { which: rightKey });
expect(spyFocusAnchorInLastTab).toHaveBeenCalled();
expect(spyFocusButtonInLastTab).toHaveBeenCalled();
lastTab.simulate('keydown', { which: leftKey });
expect(spyFocusAnchorInFirstTab).toHaveBeenCalled();
expect(spyFocusButtonInFirstTab).toHaveBeenCalled();
});

it('loops focus and selected state from lastTab to firstTab', () => {
spyFocusAnchorInFirstTab = jest.spyOn(anchorInFirstTab, 'focus');
spyFocusButtonInFirstTab = jest.spyOn(buttonInFirstTab, 'focus');
lastTab.simulate('keydown', { which: rightKey });
expect(spyFocusAnchorInFirstTab).toHaveBeenCalled();
expect(spyFocusButtonInFirstTab).toHaveBeenCalled();
});

it('loops focus and selected state from firstTab to lastTab', () => {
spyFocusAnchorInLastTab = jest.spyOn(anchorInLastTab, 'focus');
spyFocusButtonInLastTab = jest.spyOn(buttonInLastTab, 'focus');
firstTab.simulate('keydown', { which: leftKey });
expect(spyFocusAnchorInLastTab).toHaveBeenCalled();
expect(spyFocusButtonInLastTab).toHaveBeenCalled();
});

it('updates selected state when pressing space or enter key', () => {
Expand Down Expand Up @@ -213,29 +213,29 @@ describe('Tabs', () => {
);
firstTab = wrapper.find('.firstTab').last();
lastTab = wrapper.find('.lastTab').last();
anchorInFirstTab = firstTab.find('a').getDOMNode();
anchorInLastTab = lastTab.find('a').getDOMNode();
buttonInFirstTab = firstTab.find('button').getDOMNode();
buttonInLastTab = lastTab.find('button').getDOMNode();
});
it('updates selected state when pressing arrow keys', () => {
spyFocusAnchorInFirstTab = jest.spyOn(anchorInFirstTab, 'focus');
spyFocusAnchorInLastTab = jest.spyOn(anchorInLastTab, 'focus');
spyFocusButtonInFirstTab = jest.spyOn(buttonInFirstTab, 'focus');
spyFocusButtonInLastTab = jest.spyOn(buttonInLastTab, 'focus');
firstTab.simulate('keydown', { which: rightKey });
expect(spyFocusAnchorInLastTab).toHaveBeenCalled();
expect(spyFocusButtonInLastTab).toHaveBeenCalled();
lastTab.simulate('keydown', { which: leftKey });
expect(spyFocusAnchorInFirstTab).toHaveBeenCalled();
expect(spyFocusButtonInFirstTab).toHaveBeenCalled();
});

it('loops focus and selected state from lastTab to firstTab', () => {
spyFocusAnchorInFirstTab = jest.spyOn(anchorInFirstTab, 'focus');
spyFocusButtonInFirstTab = jest.spyOn(buttonInFirstTab, 'focus');
wrapper.setState({ selected: 2 });
lastTab.simulate('keydown', { which: rightKey });
expect(spyFocusAnchorInFirstTab).toHaveBeenCalled();
expect(spyFocusButtonInFirstTab).toHaveBeenCalled();
});

it('loops focus and selected state from firstTab to lastTab', () => {
spyFocusAnchorInLastTab = jest.spyOn(anchorInLastTab, 'focus');
spyFocusButtonInLastTab = jest.spyOn(buttonInLastTab, 'focus');
firstTab.simulate('keydown', { which: leftKey });
expect(spyFocusAnchorInLastTab).toHaveBeenCalled();
expect(spyFocusButtonInLastTab).toHaveBeenCalled();
});

it('updates selected state when pressing space or enter key', () => {
Expand All @@ -247,13 +247,13 @@ describe('Tabs', () => {
});

afterEach(() => {
if (spyFocusAnchorInLastTab) {
spyFocusAnchorInLastTab.mockRestore();
spyFocusAnchorInLastTab = null;
if (spyFocusButtonInLastTab) {
spyFocusButtonInLastTab.mockRestore();
spyFocusButtonInLastTab = null;
}
if (spyFocusAnchorInFirstTab) {
spyFocusAnchorInFirstTab.mockRestore();
spyFocusAnchorInFirstTab = null;
if (spyFocusButtonInFirstTab) {
spyFocusButtonInFirstTab.mockRestore();
spyFocusButtonInFirstTab = null;
}
});
});
Expand Down
Loading