Skip to content

Commit

Permalink
fix: add aria attributes to popover control
Browse files Browse the repository at this point in the history
* update tests and snapshots
  • Loading branch information
prsdthkr committed Nov 20, 2019
1 parent 90133fe commit 3e8bea3
Show file tree
Hide file tree
Showing 13 changed files with 174 additions and 71 deletions.
10 changes: 5 additions & 5 deletions src/Dropdown/Dropdown.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('<Dropdown />', () => {
<Popover
body={defaultMenu}
control={<Button className='fd-dropdown__control'>Select</Button>}
noArrow />
noArrow popperProps={{ id: 'fd-default-dropdown-popover' }} />
</Dropdown>
);

Expand All @@ -36,7 +36,7 @@ describe('<Dropdown />', () => {
Select
</Button>
}
noArrow />
noArrow popperProps={{ id: 'fd-compact-dropdown-popover' }} />
</Dropdown>
);

Expand All @@ -49,7 +49,7 @@ describe('<Dropdown />', () => {
Select
</Button>
}
noArrow />
noArrow popperProps={{ id: 'fd-toolbar-dropdown-popover' }} />
</Dropdown>
);

Expand All @@ -63,7 +63,7 @@ describe('<Dropdown />', () => {
</Button>
}
id='jhqD0557'
noArrow />
noArrow popperProps={{ id: 'fd-icon-dropdown-popover' }} />
</Dropdown>
);

Expand All @@ -80,7 +80,7 @@ describe('<Dropdown />', () => {
}
disabled
id='jhqD0561'
noArrow />
noArrow popperProps={{ id: 'fd-disable-dropdown-popover' }} />
</Dropdown>
);

Expand Down
10 changes: 10 additions & 0 deletions src/Dropdown/__snapshots__/Dropdown.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ exports[`<Dropdown /> create dropdown component 1`] = `
className="fd-popover__control"
>
<button
aria-controls="fd-default-dropdown-popover"
aria-expanded={false}
aria-haspopup={true}
className="fd-button fd-dropdown__control"
onClick={[Function]}
Expand Down Expand Up @@ -47,6 +49,8 @@ exports[`<Dropdown /> create dropdown component 2`] = `
className="fd-popover__control"
>
<button
aria-controls="fd-compact-dropdown-popover"
aria-expanded={false}
aria-haspopup={true}
className="fd-button fd-button--compact fd-dropdown__control"
onClick={[Function]}
Expand Down Expand Up @@ -78,6 +82,8 @@ exports[`<Dropdown /> create dropdown component 3`] = `
className="fd-popover__control"
>
<button
aria-controls="fd-toolbar-dropdown-popover"
aria-expanded={false}
aria-haspopup={true}
className="fd-button fd-dropdown__control"
onClick={[Function]}
Expand Down Expand Up @@ -110,6 +116,8 @@ exports[`<Dropdown /> create dropdown component 4`] = `
className="fd-popover__control"
>
<button
aria-controls="fd-icon-dropdown-popover"
aria-expanded={false}
aria-haspopup={true}
className="fd-button sap-icon--filter fd-dropdown__control"
onClick={[Function]}
Expand Down Expand Up @@ -142,6 +150,8 @@ exports[`<Dropdown /> create dropdown component 5`] = `
className="fd-popover__control"
>
<button
aria-controls="fd-disable-dropdown-popover"
aria-expanded={false}
aria-haspopup={true}
className="fd-button sap-icon--filter is-disabled fd-dropdown__control"
disabled={true}
Expand Down
24 changes: 20 additions & 4 deletions src/Popover/Popover.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ import chain from 'chain-function';
import classnames from 'classnames';
import keycode from 'keycode';
import Popper from '../utils/_Popper';
import { POPPER_PLACEMENTS } from '../utils/constants';
import PropTypes from 'prop-types';
import shortId from '../utils/shortId';
import withStyles from '../utils/WithStyles/WithStyles';
import { POPOVER_TYPES, POPPER_PLACEMENTS } from '../utils/constants';
import React, { Component } from 'react';

class Popover extends Component {
Expand All @@ -14,6 +15,10 @@ class Popover extends Component {
this.state = {
isExpanded: false
};

//A generated shortId as fallback, incase props.popperProps.id is unset.
//This ID binds the popover and it's control by 'aria-controls'.
this.ariaControls = shortId.generate();
}

isButton = (node) => {
Expand Down Expand Up @@ -71,6 +76,7 @@ class Popover extends Component {
className,
placement,
popperProps,
type,
...rest
} = this.props;

Expand All @@ -79,6 +85,12 @@ class Popover extends Component {
onClickFunctions = chain(this.triggerBody, control.props.onClick);
}

let newPopperProps = !!popperProps ? popperProps : {};
// eslint-disable-next-line no-undefined
if (newPopperProps.id === undefined || newPopperProps.id === null) {
Object.assign(newPopperProps, { id: this.ariaControls });
}

let controlProps = {
onClick: onClickFunctions
};
Expand All @@ -88,7 +100,9 @@ class Popover extends Component {
...controlProps,
tabIndex: 0,
role: 'button',
'aria-haspopup': true,
'aria-controls': newPopperProps.id,
'aria-expanded': this.state.isExpanded,
'aria-haspopup': !!type ? type : true,
onKeyPress: (event) => this.handleKeyPress(event, control, onClickFunctions)
};
}
Expand All @@ -106,7 +120,7 @@ class Popover extends Component {
onClickOutside={chain(this.handleOutsideClick, onClickOutside)}
onEscapeKey={chain(this.handleOutsideClick, onEscapeKey)}
popperPlacement={placement}
popperProps={popperProps}
popperProps={newPopperProps}
referenceClassName='fd-popover__control'
referenceComponent={referenceComponent}
show={this.state.isExpanded && !disabled}
Expand All @@ -132,6 +146,7 @@ Popover.propTypes = {
noArrow: PropTypes.bool,
placement: PropTypes.oneOf(POPPER_PLACEMENTS),
popperProps: PropTypes.object,
type: PropTypes.oneOf(POPOVER_TYPES),
onClickOutside: PropTypes.func,
onEscapeKey: PropTypes.func
};
Expand All @@ -150,7 +165,8 @@ Popover.propDescriptions = {
placement: 'Initial position of the `body` (overlay) related to the `control`.',
popperProps: 'Additional props to be spread to the overlay element, supported by <a href="https://popper.js.org" target="_blank">popper.js</a>.',
onClickOutside: 'Callback for consumer clicking outside of popover body.',
onEscapeKey: 'Callback when escape key is pressed when popover body is visible.'
onEscapeKey: 'Callback when escape key is pressed when popover body is visible.',
type: 'Indicates the type of popup - "dialog", "grid", "listbox", "menu", or "tree". This value is attached to aria-haspopup and is useful to assistive tech.'
};

export { Popover as __Popover };
Expand Down
10 changes: 5 additions & 5 deletions src/Popover/Popover.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('<Popover />', () => {
</Menu.List>
</Menu>
}
control={<Icon glyph='cart' size='xl' />} />
control={<Icon glyph='cart' size='xl' />} popperProps={{ id: 'fd-default-popover' }} />
);

const popOverDisabled = (
Expand All @@ -36,7 +36,7 @@ describe('<Popover />', () => {
}
className='blue'
control={<Icon glyph='cart' size='xl' />}
disabled />
disabled popperProps={{ id: 'fd-disabled-popover' }} />
);

const popOverWithAlignment = (
Expand All @@ -52,7 +52,7 @@ describe('<Popover />', () => {
</Menu>
}
control={<Icon glyph='cart' size='xl' />}
placement='right' />
placement='right' popperProps={{ id: 'fd-aligned-popover' }} />
);

const popOverNoArrow = (
Expand All @@ -68,7 +68,7 @@ describe('<Popover />', () => {
</Menu>
}
control={<Icon glyph='cart' size='xl' />}
noArrow />
noArrow popperProps={{ id: 'fd-arrowless-popover' }} />
);

const popOverDisableEdgeDetection = (
Expand All @@ -84,7 +84,7 @@ describe('<Popover />', () => {
</Menu>
}
control={<Icon glyph='cart' size='xl' />}
disableEdgeDetection />
disableEdgeDetection popperProps={{ id: 'fd-edge-undetected-popover' }} />
);

test('create Popover', () => {
Expand Down
8 changes: 8 additions & 0 deletions src/Popover/__snapshots__/Popover.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ exports[`<Popover /> create Popover 1`] = `
className="fd-popover__control"
>
<span
aria-controls="fd-default-popover"
aria-expanded={false}
aria-haspopup={true}
className="sap-icon--cart sap-icon--xl"
onClick={[Function]}
Expand All @@ -38,6 +40,8 @@ exports[`<Popover /> create Popover 2`] = `
className="fd-popover__control"
>
<span
aria-controls="fd-disabled-popover"
aria-expanded={false}
aria-haspopup={true}
className="sap-icon--cart sap-icon--xl"
onClick={[Function]}
Expand All @@ -63,6 +67,8 @@ exports[`<Popover /> create Popover 3`] = `
className="fd-popover__control"
>
<span
aria-controls="fd-aligned-popover"
aria-expanded={false}
aria-haspopup={true}
className="sap-icon--cart sap-icon--xl"
onClick={[Function]}
Expand All @@ -88,6 +94,8 @@ exports[`<Popover /> create Popover 4`] = `
className="fd-popover__control"
>
<span
aria-controls="fd-arrowless-popover"
aria-expanded={false}
aria-haspopup={true}
className="sap-icon--cart sap-icon--xl"
onClick={[Function]}
Expand Down
Loading

0 comments on commit 3e8bea3

Please sign in to comment.