Skip to content

Combobox: Set role='combobox' on input element #2756

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

Merged
interactivellama merged 4 commits into from
Feb 5, 2021
Merged

Combobox: Set role='combobox' on input element #2756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a530b95
Combobox: Set role=\'combobox\' on input element
bxiao Feb 4, 2021
2a1aaf6
add test
bxiao Feb 4, 2021
38635ef
update snapshots
bxiao Feb 4, 2021
41f0cf5
move more aria attributes to input
bxiao Feb 5, 2021
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
383 changes: 163 additions & 220 deletions components/combobox/__docs__/__snapshots__/storybook-stories.storyshot
View file
Open in desktop

Large diffs are not rendered by default.

9 changes: 5 additions & 4 deletions components/combobox/__tests__/combobox.browser-test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,15 +226,16 @@ describe('SLDSCombobox', function describeFunction() {
destroyMountNode({ wrapper, mountNode });
});

it('has aria-haspopup, aria-expanded is false when closed, aria-expanded is true when open', function () {
it('has aria-haspopup, role is combobox, aria-expanded is false when closed, aria-expanded is true when open', function () {
wrapper = mount(<DemoComponent multiple />, { attachTo: mountNode });
const nodes = getNodes({ wrapper });
expect(nodes.combobox).attr('aria-haspopup', 'listbox');
expect(nodes.input).attr('aria-haspopup', 'listbox');
expect(nodes.input).attr('role', 'combobox');
// closed
expect(nodes.combobox).attr('aria-expanded', 'false');
expect(nodes.input).attr('aria-expanded', 'false');
// open
nodes.input.simulate('click', {});
expect(nodes.combobox).attr('aria-expanded', 'true');
expect(nodes.input).attr('aria-expanded', 'true');
});

it('menu filters to second item, menu listbox menu item 2 aria-selected is true, input activedescendent has item 2 id, after pressing down arrow, enter selects item 2', function () {
Expand Down
42 changes: 18 additions & 24 deletions components/combobox/combobox.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1074,11 +1074,8 @@ class Combobox extends React.Component {
},
props.className
)}
aria-expanded={this.getIsOpen()}
aria-haspopup="listbox" // eslint-disable-line jsx-a11y/aria-proptypes
// used on menu's listbox
aria-owns={this.getIsOpen() ? `${this.getId()}-listbox` : undefined} // eslint-disable-line jsx-a11y/aria-proptypes
role="combobox"
>
<InnerInput
aria-autocomplete="list"
Expand All @@ -1091,6 +1088,9 @@ class Combobox extends React.Component {
: null
}
aria-describedby={this.getErrorId()}
aria-expanded={this.getIsOpen()}
aria-haspopup="listbox" // eslint-disable-line jsx-a11y/aria-proptypes
role="combobox"
autoComplete="off"
className="slds-combobox__input"
containerProps={{
Expand Down Expand Up @@ -1120,7 +1120,6 @@ class Combobox extends React.Component {
!!(props.predefinedOptionsOnly && this.state.activeOption)
}
required={props.required}
role="textbox"
value={
props.predefinedOptionsOnly
? (this.state.activeOption && this.state.activeOption.label) ||
Expand Down Expand Up @@ -1215,9 +1214,6 @@ class Combobox extends React.Component {
},
props.className
)}
aria-expanded={this.getIsOpen()}
aria-haspopup="listbox" // eslint-disable-line jsx-a11y/aria-proptypes
role="combobox"
>
<InnerInput
aria-autocomplete="list"
Expand All @@ -1230,6 +1226,9 @@ class Combobox extends React.Component {
: null
}
aria-describedby={this.getErrorId()}
aria-expanded={this.getIsOpen()}
aria-haspopup="listbox" // eslint-disable-line jsx-a11y/aria-proptypes
role="combobox"
defaultValue={props.defaultValue}
autoComplete="off"
className="slds-combobox__input"
Expand Down Expand Up @@ -1259,7 +1258,6 @@ class Combobox extends React.Component {
!!(props.predefinedOptionsOnly && this.state.activeOption)
}
required={props.required}
role="textbox"
value={
props.predefinedOptionsOnly
? (this.state.activeOption && this.state.activeOption.label) ||
Expand Down Expand Up @@ -1316,9 +1314,6 @@ class Combobox extends React.Component {
},
props.className
)}
aria-expanded={this.getIsOpen()}
aria-haspopup="listbox" // eslint-disable-line jsx-a11y/aria-proptypes
role="combobox"
>
<InnerInput
defaultValue={props.defaultValue}
Expand All @@ -1334,6 +1329,9 @@ class Combobox extends React.Component {
: null
}
aria-describedby={this.getErrorId()}
aria-expanded={this.getIsOpen()}
aria-haspopup="listbox" // eslint-disable-line jsx-a11y/aria-proptypes
role="combobox"
autoComplete="off"
className="slds-combobox__input"
containerProps={{
Expand Down Expand Up @@ -1384,7 +1382,6 @@ class Combobox extends React.Component {
!!props.selection.length
}
required={props.required}
role="textbox"
value={
props.predefinedOptionsOnly
? (this.state.activeOption &&
Expand Down Expand Up @@ -1481,9 +1478,6 @@ class Combobox extends React.Component {
},
props.className
)}
aria-expanded={this.getIsOpen()}
aria-haspopup="dialog" // eslint-disable-line jsx-a11y/aria-proptypes
role="combobox"
>
<Popover {...popoverProps}>
<InnerInput
Expand All @@ -1492,6 +1486,9 @@ class Combobox extends React.Component {
this.getIsOpen() ? `${this.getId()}-popover` : undefined
}
aria-describedby={this.getErrorId()}
aria-expanded={this.getIsOpen()}
aria-haspopup="dialog" // eslint-disable-line jsx-a11y/aria-proptypes
role="combobox"
autoComplete="off"
className="slds-combobox__input"
containerProps={{
Expand All @@ -1517,7 +1514,6 @@ class Combobox extends React.Component {
placeholder={labels.placeholder}
readOnly
required={props.required}
role="textbox"
value={props.value}
/>
</Popover>
Expand Down Expand Up @@ -1567,9 +1563,6 @@ class Combobox extends React.Component {
},
props.className
)}
aria-expanded={this.getIsOpen()}
aria-haspopup="listbox" // eslint-disable-line jsx-a11y/aria-proptypes
role="combobox"
>
<InnerInput
defaultValue={props.defaultValue}
Expand All @@ -1585,6 +1578,9 @@ class Combobox extends React.Component {
: null
}
aria-describedby={this.getErrorId()}
aria-expanded={this.getIsOpen()}
aria-haspopup="listbox" // eslint-disable-line jsx-a11y/aria-proptypes
role="combobox"
autoComplete="off"
className="slds-combobox__input"
containerProps={{
Expand All @@ -1610,7 +1606,6 @@ class Combobox extends React.Component {
placeholder={labels.placeholderReadOnly}
readOnly
required={props.required}
role="textbox"
value={value}
{...userDefinedProps.input}
/>
Expand Down Expand Up @@ -1679,9 +1674,6 @@ class Combobox extends React.Component {
},
props.className
)}
aria-expanded={this.getIsOpen()}
aria-haspopup="listbox" // eslint-disable-line jsx-a11y/aria-proptypes
role="combobox"
>
<InnerInput
defaultValue={props.defaultValue}
Expand All @@ -1697,6 +1689,9 @@ class Combobox extends React.Component {
: null
}
aria-describedby={this.getErrorId()}
aria-expanded={this.getIsOpen()}
aria-haspopup="listbox" // eslint-disable-line jsx-a11y/aria-proptypes
role="combobox"
autoComplete="off"
className="slds-combobox__input"
containerProps={{
Expand All @@ -1723,7 +1718,6 @@ class Combobox extends React.Component {
placeholder={labels.placeholderReadOnly}
readOnly
required={props.required}
role="textbox"
value={
(this.state.activeOption && this.state.activeOption.label) ||
value
Expand Down
Loading