Unverified Commit 92cdc422 authored by tragessere's avatar tragessere Committed by GitHub
Browse files

Fix/dropdown accessibility (#4977) (#4978)

* Allow id and aria-controls props to pass through AbstractNavItem if navContext doesn't provide a value.

* Fix documentation typo

* Add aria-controls to NavDropdown and always render the dropdown menu.

* Add tests for the dropdown id and aria-controls changes

* Add aria-controls to DropdownToggle in DropdownButton and SplitButton

* Add tests for aria-controls attribute on DropdownButton and SplitButton

* Fix test name typo

* Add tests for aria-controls attribute on DropdownButton and SplitButton

* Add prop to render dropdown menus in the DOM before being shown

* Display a warning if user's id is ignored in AbstractNavItem.

Also pass the user provided id through to Tab children so the inner AbstractNavItem knows the user's id is being overwritten.
parent 0a50fa3d
......@@ -3,10 +3,12 @@ import PropTypes from 'prop-types';
import React, { useContext } from 'react';
import useEventCallback from '@restart/hooks/useEventCallback';
import warning from 'warning';
import NavContext from './NavContext';
import SelectableContext, { makeEventKey } from './SelectableContext';
const propTypes = {
id: PropTypes.string,
active: PropTypes.bool,
role: PropTypes.string,
......@@ -18,6 +20,8 @@ const propTypes = {
as: PropTypes.any,
onClick: PropTypes.func,
onSelect: PropTypes.func,
'aria-controls': PropTypes.string,
};
const defaultProps = {
......@@ -46,9 +50,21 @@ const AbstractNavItem = React.forwardRef(
if (navContext) {
if (!props.role && navContext.role === 'tablist') props.role = 'tab';
const contextControllerId = navContext.getControllerId(navKey);
const contextControlledId = navContext.getControlledId(navKey);
warning(
!contextControllerId || !props.id,
`[react-bootstrap] The provided id '${props.id}' was overwritten by the current navContext with '${contextControllerId}'.`,
);
warning(
!contextControlledId || !props['aria-controls'],
`[react-bootstrap] The provided aria-controls value '${props['aria-controls']}' was overwritten by the current navContext with '${contextControlledId}'.`,
);
props['data-rb-event-key'] = navKey;
props.id = navContext.getControllerId(navKey);
props['aria-controls'] = navContext.getControlledId(navKey);
props.id = contextControllerId || props.id;
props['aria-controls'] = contextControlledId || props['aria-controls'];
isActive =
active == null && navKey != null
......
......@@ -26,6 +26,9 @@ const propTypes = {
/** An ARIA accessible role applied to the Menu component. When set to 'menu', The dropdown */
menuRole: PropTypes.string,
/** Whether to render the dropdown menu in the DOM before the first time it is shown */
renderMenuOnMount: PropTypes.bool,
/**
* Which event when fired outside the component will cause it to be closed.
*
......@@ -59,6 +62,7 @@ const DropdownButton = React.forwardRef(
variant,
size,
menuRole,
renderMenuOnMount,
disabled,
href,
id,
......@@ -77,7 +81,11 @@ const DropdownButton = React.forwardRef(
>
{title}
</Dropdown.Toggle>
<Dropdown.Menu role={menuRole} rootCloseEvent={rootCloseEvent}>
<Dropdown.Menu
role={menuRole}
renderOnMount={renderMenuOnMount}
rootCloseEvent={rootCloseEvent}
>
{children}
</Dropdown.Menu>
</Dropdown>
......
......@@ -16,6 +16,9 @@ const propTypes = {
/** Controls the visibility of the Dropdown menu */
show: PropTypes.bool,
/** Whether to render the dropdown menu in the DOM before the first time it is shown */
renderOnMount: PropTypes.bool,
/** Have the dropdown switch to it's opposite placement when necessary to stay on screen. */
flip: PropTypes.bool,
......@@ -63,6 +66,7 @@ const DropdownMenu = React.forwardRef(
flip,
popperConfig,
show: showProps,
renderOnMount,
// Need to define the default "as" during prop destructuring to be compatible with styled-components github.com/react-bootstrap/react-bootstrap/issues/3595
as: Component = 'div',
...props
......@@ -92,7 +96,7 @@ const DropdownMenu = React.forwardRef(
useWrappedRefWithWarning(ref, 'DropdownMenu'),
);
if (!hasShown) return null;
if (!hasShown && !renderOnMount) return null;
// For custom components provide additional, non-DOM, props;
if (typeof Component !== 'string') {
......
......@@ -28,6 +28,9 @@ const propTypes = {
/** An ARIA accessible role applied to the Menu component. When set to 'menu', The dropdown */
menuRole: PropTypes.string,
/** Whether to render the dropdown menu in the DOM before the first time it is shown */
renderMenuOnMount: PropTypes.bool,
/**
* Which event when fired outside the component will cause it to be closed.
*
......@@ -50,6 +53,7 @@ const NavDropdown = React.forwardRef(
menuRole,
disabled,
active,
renderMenuOnMount,
...props
},
ref,
......@@ -66,7 +70,11 @@ const NavDropdown = React.forwardRef(
{title}
</Dropdown.Toggle>
<Dropdown.Menu role={menuRole} rootCloseEvent={rootCloseEvent}>
<Dropdown.Menu
role={menuRole}
renderOnMount={renderMenuOnMount}
rootCloseEvent={rootCloseEvent}
>
{children}
</Dropdown.Menu>
</Dropdown>
......
......@@ -38,6 +38,10 @@ const propTypes = {
/** An ARIA accessible role applied to the Menu component. When set to 'menu', The dropdown */
menuRole: PropTypes.string,
/** Whether to render the dropdown menu in the DOM before the first time it is shown */
renderMenuOnMount: PropTypes.bool,
/**
* Which event when fired outside the component will cause it to be closed.
*
......@@ -73,6 +77,7 @@ const SplitButton = React.forwardRef(
href,
target,
menuRole,
renderMenuOnMount,
rootCloseEvent,
...props
},
......@@ -102,7 +107,11 @@ const SplitButton = React.forwardRef(
<span className="sr-only">{toggleLabel}</span>
</Dropdown.Toggle>
<Dropdown.Menu role={menuRole} rootCloseEvent={rootCloseEvent}>
<Dropdown.Menu
role={menuRole}
renderOnMount={renderMenuOnMount}
rootCloseEvent={rootCloseEvent}
>
{children}
</Dropdown.Menu>
</Dropdown>
......
......@@ -94,7 +94,7 @@ function getDefaultActiveKey(children) {
}
function renderTab(child) {
const { title, eventKey, disabled, tabClassName } = child.props;
const { title, eventKey, disabled, tabClassName, id } = child.props;
if (title == null) {
return null;
}
......@@ -104,6 +104,7 @@ function renderTab(child) {
as={NavLink}
eventKey={eventKey}
disabled={disabled}
id={id}
className={tabClassName}
>
{title}
......
......@@ -33,6 +33,14 @@ describe('<Dropdown.Menu>', () => {
).assertSingle('.dropdown-menu-right');
});
it('renders on mount with prop', () => {
mount(
<DropdownMenu renderOnMount>
<DropdownItem>Item</DropdownItem>
</DropdownMenu>,
).assertSingle('div.dropdown-menu');
});
// it.only('warns about bad refs', () => {
// class Parent extends React.Component {
// componentDidCatch() {}
......
......@@ -49,4 +49,14 @@ describe('<NavDropdown>', () => {
.text()
.should.equal('DropdownItem 2 content');
});
it('should pass the id to the NavLink element', () => {
const wrapper = mount(
<NavDropdown id="test-id" title="title">
<DropdownItem eventKey="1">DropdownItem 1 content</DropdownItem>
</NavDropdown>,
);
wrapper.assertSingle('a#test-id');
});
});
......@@ -16,6 +16,7 @@ export interface DropdownButtonProps extends PropsFromToggle {
id: string;
title: React.ReactNode;
menuRole?: string;
renderMenuOnMount?: boolean;
rootCloseEvent?: 'click' | 'mousedown';
bsPrefix?: string;
}
......
......@@ -4,6 +4,7 @@ import { BsPrefixComponent, SelectCallback } from './helpers';
export interface DropdownMenuProps {
show?: boolean;
renderOnMount?: boolean;
flip?: boolean;
alignRight?: boolean;
onSelect?: SelectCallback;
......
......@@ -10,6 +10,7 @@ export interface NavDropdownProps {
disabled?: boolean;
active?: boolean;
menuRole?: string;
renderMenuOnMount?: boolean;
rootCloseEvent?: 'click' | 'mousedown';
bsPrefix?: string;
}
......
......@@ -20,6 +20,7 @@ export interface SplitButtonProps extends PropsFromToggle {
onClick?: React.MouseEventHandler<this>;
title: React.ReactNode;
menuRole?: string;
renderMenuOnMount?: boolean;
rootCloseEvent?: 'click' | 'mousedown';
bsPrefix?: string;
}
......
......@@ -116,7 +116,7 @@ component
<CodeBlock codeText={NavDropdownImpl} />
The above demostrates how flexible the component model can be. But if
The above demonstrates how flexible the component model can be. But if
you didn't want to roll your own versions we've included a
straight-forward `<NavDropdown>` that works for most cases.
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment