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

feat: add accessibility features to popover control #792

Merged
merged 3 commits into from
Nov 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/ComboboxInput/ComboboxInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const ComboboxInput = React.forwardRef(({ placeholder, menu, compact, className,
</span>
</div>
}
disableKeyPressHandler
disableStyles={disableStyles}
noArrow />
</div>
Expand Down
20 changes: 20 additions & 0 deletions src/Dropdown/__snapshots__/Dropdown.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,12 @@ exports[`<Dropdown /> create dropdown component 1`] = `
className="fd-popover__control"
>
<button
aria-haspopup={true}
className="fd-button fd-dropdown__control"
onClick={[Function]}
onKeyPress={[Function]}
role="button"
tabIndex={0}
>
Select
</button>
Expand All @@ -43,8 +47,12 @@ exports[`<Dropdown /> create dropdown component 2`] = `
className="fd-popover__control"
>
<button
aria-haspopup={true}
className="fd-button fd-button--compact fd-dropdown__control"
onClick={[Function]}
onKeyPress={[Function]}
role="button"
tabIndex={0}
>
Select
</button>
Expand All @@ -70,8 +78,12 @@ exports[`<Dropdown /> create dropdown component 3`] = `
className="fd-popover__control"
>
<button
aria-haspopup={true}
className="fd-button fd-dropdown__control"
onClick={[Function]}
onKeyPress={[Function]}
role="button"
tabIndex={0}
>
Select
</button>
Expand All @@ -98,8 +110,12 @@ exports[`<Dropdown /> create dropdown component 4`] = `
className="fd-popover__control"
>
<button
aria-haspopup={true}
className="fd-button sap-icon--filter fd-dropdown__control"
onClick={[Function]}
onKeyPress={[Function]}
role="button"
tabIndex={0}
>
Select
</button>
Expand All @@ -126,9 +142,13 @@ exports[`<Dropdown /> create dropdown component 5`] = `
className="fd-popover__control"
>
<button
aria-haspopup={true}
className="fd-button sap-icon--filter is-disabled fd-dropdown__control"
disabled={true}
onClick={[Function]}
onKeyPress={[Function]}
role="button"
tabIndex={0}
>
Select
</button>
Expand Down
11 changes: 9 additions & 2 deletions src/Identifier/Identifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import React from 'react';
import withStyles from '../utils/WithStyles/WithStyles';
import { IDENTIFIER_MODIFIERS, IDENTIFIER_SIZES } from '../utils/constants';

const Identifier = React.forwardRef(({ glyph, size, modifier, color, label, backgroundImageUrl, children, className, disableStyles, ...props }, ref) => {
const Identifier = React.forwardRef(({ glyph, size, modifier, color, label, backgroundImageUrl, children, className, disableStyles, role, ...props }, ref) => {
const styles = {
backgroundImage: `url(${backgroundImageUrl})`
};
Expand All @@ -21,7 +21,12 @@ const Identifier = React.forwardRef(({ glyph, size, modifier, color, label, back
className
);

const ariaRole = !children ? 'presentation' : '';
let ariaRole;
if (role) {
ariaRole = role;
} else {
ariaRole = !children ? 'presentation' : '';
}

return (
<span
Expand All @@ -48,13 +53,15 @@ Identifier.propTypes = {
glyph: PropTypes.string,
label: PropTypes.string,
modifier: PropTypes.oneOf(IDENTIFIER_MODIFIERS),
role: PropTypes.string,
size: PropTypes.oneOf(IDENTIFIER_SIZES)
};

Identifier.propDescriptions = {
backgroundImageUrl: 'Image URL.',
color: 'Applies a background color.',
label: 'Localized text for label.',
role: 'Applies an aria-role. Set to button if Identifier opens a Popover or Modal.',
size: 'Size of the image. These sizes are available: **xxs** (extra extra small) - 20px, **xs** (extra small) - 28px, **s** (small) - 32px, **m** (medium) - 48px, **l** (large) - 64px, **xl** (extra lagre) - 88px, and **xxl** (extra extra large). Default matches the base font size (14px).'
};

Expand Down
1 change: 1 addition & 0 deletions src/LocalizationEditor/LocalizationEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ const LocalizationEditor = React.forwardRef(({ control, menu, id, compact, texta
</span>
</div>
}
disableKeyPressHandler
disableStyles={disableStyles}
id={id}
noArrow />
Expand Down
45 changes: 43 additions & 2 deletions src/Popover/Popover.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
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';
Expand All @@ -15,6 +16,18 @@ class Popover extends Component {
};
}

isButton = (node) => {
if (!node) {
return false;
}
if (typeof node.type === 'string') {
return node.type.toLowerCase() === 'button';
} else if (node.type.displayName) {
return node.type.displayName.toLowerCase() === 'button';
}
return false;
};

triggerBody = () => {
if (!this.props.disabled) {
this.setState(prevState => ({
Expand All @@ -31,9 +44,23 @@ class Popover extends Component {
}
};

handleKeyPress = (event, node, onClickFunctions) => {
if (!this.isButton(node)) {
switch (keycode(event)) {
case 'enter':
case 'space':
event.preventDefault();
onClickFunctions();
break;
default:
}
}
}

render() {
const {
disableEdgeDetection,
disableKeyPressHandler,
disableStyles,
onClickOutside,
onEscapeKey,
Expand All @@ -52,9 +79,21 @@ class Popover extends Component {
onClickFunctions = chain(this.triggerBody, control.props.onClick);
}

const referenceComponent = React.cloneElement(control, {
let controlProps = {
onClick: onClickFunctions
});
};

if (!disableKeyPressHandler) {
controlProps = {
prsdthkr marked this conversation as resolved.
Show resolved Hide resolved
...controlProps,
tabIndex: 0,
role: 'button',
'aria-haspopup': true,
onKeyPress: (event) => this.handleKeyPress(event, control, onClickFunctions)
};
}

const referenceComponent = React.cloneElement(control, controlProps);

const popoverClasses = classnames('fd-popover', className);

Expand Down Expand Up @@ -88,6 +127,7 @@ Popover.propTypes = {
customStyles: PropTypes.object,
disabled: PropTypes.bool,
disableEdgeDetection: PropTypes.bool,
disableKeyPressHandler: PropTypes.bool,
disableStyles: PropTypes.bool,
noArrow: PropTypes.bool,
placement: PropTypes.oneOf(POPPER_PLACEMENTS),
Expand All @@ -105,6 +145,7 @@ Popover.propDescriptions = {
body: 'Node(s) to render in the overlay.',
control: 'Node to render as the reference element (that the `body` will be placed in relation to).',
disableEdgeDetection: 'Set to **true** to render popover without edge detection so popover will not flip from top to bottom with scroll.',
disableKeyPressHandler: 'Set to **true** to remove onKeyPress handler and aria-* roles. Only do so if the control is a complex component such as a FormInput with Button.',
noArrow: 'Set to **true** to render a popover without an arrow.',
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>.',
Expand Down
47 changes: 47 additions & 0 deletions src/Popover/Popover.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,53 @@ describe('<Popover />', () => {
});
});

describe('control accessibility', () => {
test('adds a tabindex of 0 to the control', () => {
const wrapper = mountComponentWithStyles(popOver);
const button = wrapper.find('Icon').at(0);

expect(button.props().tabIndex).toEqual(0);
});

test('adds aria-haspopup to the control', () => {
const wrapper = mountComponentWithStyles(popOver);
const button = wrapper.find('Icon').at(0);

expect(button.props()['aria-haspopup']).toEqual(true);
});

test('adds a role of button to the control', () => {
const wrapper = mountComponentWithStyles(popOver);
const button = wrapper.find('Icon').at(0);

expect(button.props().role).toEqual('button');
});

test('handle space key to open popover', () => {
const syntheticEvent = {
keyCode: 32,
preventDefault: () => {}
};
const wrapper = mountComponentWithStyles(popOver);
const button = wrapper.find('Icon').at(0);
button.prop('onKeyPress')(syntheticEvent, 'Icon', wrapper.triggerBody);

expect(wrapper.state('isExpanded')).toBeTruthy();
});

test('handle enter key to open popover', () => {
const syntheticEvent = {
keyCode: 13,
preventDefault: () => {}
};
const wrapper = mountComponentWithStyles(popOver);
const button = wrapper.find('Icon').at(0);
button.prop('onKeyPress')(syntheticEvent, 'Icon', wrapper.triggerBody);

expect(wrapper.state('isExpanded')).toBeTruthy();
});
});

describe('Callback handler', () => {
test('should dispatch the onClickOutside callback with the event', () => {
let f = jest.fn();
Expand Down
16 changes: 16 additions & 0 deletions src/Popover/__snapshots__/Popover.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ exports[`<Popover /> create Popover 1`] = `
className="fd-popover__control"
>
<span
aria-haspopup={true}
className="sap-icon--cart sap-icon--xl"
onClick={[Function]}
onKeyPress={[Function]}
role="button"
tabIndex={0}
/>
</div>
</div>
Expand All @@ -34,8 +38,12 @@ exports[`<Popover /> create Popover 2`] = `
className="fd-popover__control"
>
<span
aria-haspopup={true}
className="sap-icon--cart sap-icon--xl"
onClick={[Function]}
onKeyPress={[Function]}
role="button"
tabIndex={0}
/>
</div>
</div>
Expand All @@ -55,8 +63,12 @@ exports[`<Popover /> create Popover 3`] = `
className="fd-popover__control"
>
<span
aria-haspopup={true}
className="sap-icon--cart sap-icon--xl"
onClick={[Function]}
onKeyPress={[Function]}
role="button"
tabIndex={0}
/>
</div>
</div>
Expand All @@ -76,8 +88,12 @@ exports[`<Popover /> create Popover 4`] = `
className="fd-popover__control"
>
<span
aria-haspopup={true}
className="sap-icon--cart sap-icon--xl"
onClick={[Function]}
onKeyPress={[Function]}
role="button"
tabIndex={0}
/>
</div>
</div>
Expand Down
1 change: 1 addition & 0 deletions src/SearchInput/SearchInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ class SearchInput extends Component {
)}
</div>
}
disableKeyPressHandler
disableStyles={disableStyles} />
</div>
);
Expand Down
Loading