Skip to content

Commit

Permalink
feat(Dropdown): add support for listbox or menu role (reactstrap#2077)
Browse files Browse the repository at this point in the history
* feat(Dropdown, DropdownMenu, DropdownToggle): allow Dropdown to have listbox or menu role

Dropdown Menu should allow for listbox or menu roles.  Before this commit, the Dropdown could only have the menu role.  After this commit, a user can add a menuRole property to the Dropdown component which will set a corresponding aria-haspopop property on the DropdownToggle, role property on the DropdownMenu, and role property on the Dropdown Items

* feat(Dropdown, DropdownMenu, DropdownToggle): simplify code and fix bugs

Address all code cleanlieness and backwards compatibility issues brought up in PR

* feat(Dropdown, DropdownContext, DropdownMenu, DropdownItem): refactor and fix a typo

Address all issues mentioned in PR review

Co-authored-by: Kyle Tsang <6854874+kyletsang@users.noreply.github.com>
  • Loading branch information
andyseracuse and kyletsang committed Jan 28, 2021
1 parent ae4b5c5 commit fd9e988
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 7 deletions.
16 changes: 13 additions & 3 deletions src/Dropdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const propTypes = {
cssModule: PropTypes.object,
inNavbar: PropTypes.bool,
setActiveFromChild: PropTypes.bool,
menuRole: PropTypes.oneOf(['listbox', 'menu'])
};

const defaultProps = {
Expand Down Expand Up @@ -76,6 +77,7 @@ class Dropdown extends React.Component {
// Callback that should be called by DropdownMenu to provide a ref to
// a HTML tag that's used for the DropdownMenu
onMenuRef: this.handleMenuRef,
menuRole: this.props.menuRole
};
}

Expand Down Expand Up @@ -107,12 +109,19 @@ class Dropdown extends React.Component {
return this._$menuCtrl;
}

getItemType() {
if(this.context.menuRole === 'listbox') {
return 'option'
}
return 'menuitem'
}

getMenuItems() {
// In a real menu with a child DropdownMenu, `this.getMenu()` should never
// be null, but it is sometimes null in tests. To mitigate that, we just
// use `this.getContainer()` as the fallback `menuContainer`.
const menuContainer = this.getMenu() || this.getContainer();
return [].slice.call(menuContainer.querySelectorAll('[role="menuitem"]'));
return [].slice.call(menuContainer.querySelectorAll(`[role="${this.getItemType()}"]`));
}

addEvents() {
Expand Down Expand Up @@ -141,7 +150,7 @@ class Dropdown extends React.Component {
}

handleKeyDown(e) {
const isTargetMenuItem = e.target.getAttribute('role') === 'menuitem';
const isTargetMenuItem = e.target.getAttribute('role') === 'menuitem' || e.target.getAttribute('role') === 'option';
const isTargetMenuCtrl = this.getMenuCtrl() === e.target;
const isTab = keyCodes.tab === e.which;

Expand Down Expand Up @@ -177,7 +186,7 @@ class Dropdown extends React.Component {
}
}

if (this.props.isOpen && (e.target.getAttribute('role') === 'menuitem')) {
if (this.props.isOpen && isTargetMenuItem) {
if ([keyCodes.tab, keyCodes.esc].indexOf(e.which) > -1) {
this.toggle(e);
this.getMenuCtrl().focus();
Expand Down Expand Up @@ -245,6 +254,7 @@ class Dropdown extends React.Component {
active,
addonType,
tag,
menuRole,
...attrs
} = omit(this.props, ['toggle', 'disabled', 'inNavbar', 'a11y']);

Expand Down
9 changes: 8 additions & 1 deletion src/DropdownItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ class DropdownItem extends React.Component {
this.getTabIndex = this.getTabIndex.bind(this);
}

getRole() {
if(this.context.menuRole === 'listbox') {
return 'option'
}
return 'menuitem'
}

onClick(e) {
const { disabled, header, divider, text } = this.props;
if (disabled || header || divider || text) {
Expand Down Expand Up @@ -58,7 +65,7 @@ class DropdownItem extends React.Component {

render() {
const tabIndex = this.getTabIndex();
const role = tabIndex > -1 ? 'menuitem' : undefined;
const role = tabIndex > -1 ? this.getRole() : undefined;
let {
className,
cssModule,
Expand Down
11 changes: 9 additions & 2 deletions src/DropdownMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ const directionPositionMap = {

class DropdownMenu extends React.Component {

getRole() {
if(this.context.menuRole === 'listbox') {
return 'listbox'
}
return 'menu'
}

render() {
const {
className,
Expand Down Expand Up @@ -92,7 +99,7 @@ class DropdownMenu extends React.Component {
return (
<Tag
tabIndex="-1"
role="menu"
role={this.getRole()}
ref={handleRef}
{...attrs}
style={combinedStyle}
Expand All @@ -115,7 +122,7 @@ class DropdownMenu extends React.Component {
return (
<Tag
tabIndex="-1"
role="menu"
role={this.getRole()}
{...attrs}
aria-hidden={!this.context.isOpen}
className={classes}
Expand Down
8 changes: 7 additions & 1 deletion src/DropdownToggle.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ const propTypes = {
};

const defaultProps = {
'aria-haspopup': true,
color: 'secondary',
'aria-haspopup': true
};

class DropdownToggle extends React.Component {
Expand All @@ -49,6 +49,10 @@ class DropdownToggle extends React.Component {
this.context.toggle(e);
}

getRole() {
return this.context.menuRole || this.props['aria-haspopup'];
}

render() {
const { className, color, cssModule, caret, split, nav, tag, innerRef, ...props } = this.props;
const ariaLabel = props['aria-label'] || 'Toggle Dropdown';
Expand Down Expand Up @@ -87,6 +91,7 @@ class DropdownToggle extends React.Component {
className={classes}
onClick={this.onClick}
aria-expanded={this.context.isOpen}
aria-haspopup={this.getRole()}
children={children}
/>
);
Expand All @@ -102,6 +107,7 @@ class DropdownToggle extends React.Component {
className={classes}
onClick={this.onClick}
aria-expanded={this.context.isOpen}
aria-haspopup={this.getRole()}
children={children}
/>
)}
Expand Down
43 changes: 43 additions & 0 deletions src/__tests__/Dropdown.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1115,4 +1115,47 @@ describe('Dropdown', () => {
expect(dropleft.childAt(0).childAt(0).hasClass('dropleft')).toBe(true);
expect(dropright.childAt(0).childAt(0).hasClass('dropright')).toBe(true);
});

describe('menuRole prop', () => {

it('should set correct roles for children when menuRole is menu', () => {
const wrapper = mount(
<Dropdown menuRole={'menu'} toggle={toggle}>
<DropdownToggle nav caret>
Options
</DropdownToggle>
<DropdownMenu>
<DropdownItem active>
Test
</DropdownItem>
</DropdownMenu>
</Dropdown>
);

expect(wrapper.props().menuRole).toEqual('menu')
expect(wrapper.find('[aria-haspopup="menu"]').length).toEqual(1)
expect(wrapper.find('[role="menuitem"]').length).toEqual(1)
expect(wrapper.find('[role="menu"]').length).toEqual(1)
})

it('should set correct roles for children when menuRole is menu', () => {
const wrapper = mount(
<Dropdown menuRole={'listbox'} toggle={toggle}>
<DropdownToggle nav caret>
Options
</DropdownToggle>
<DropdownMenu>
<DropdownItem active>
Test
</DropdownItem>
</DropdownMenu>
</Dropdown>
);

expect(wrapper.props().menuRole).toEqual('listbox')
expect(wrapper.find('[aria-haspopup="listbox"]').length).toEqual(1)
expect(wrapper.find('[role="option"]').length).toEqual(1)
expect(wrapper.find('[role="listbox"]').length).toEqual(1)
})
})
});
1 change: 1 addition & 0 deletions types/lib/Dropdown.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export interface DropdownProps extends React.HTMLAttributes<HTMLElement> {
cssModule?: CSSModule;
inNavbar?: boolean;
setActiveFromChild?: boolean;
menuRole?: boolean | string;
}

export interface UncontrolledDropdownProps extends DropdownProps {
Expand Down

0 comments on commit fd9e988

Please sign in to comment.