[UI Framework] Fix Popover and ContextMenu bugs (#14617)

* Rename KuiPopover isFocusable prop to ownFocus. Focus on first focusable element by default.
* Fix bug where ContextMenuPanel keyboard navigation broke if the user was using tab instead of arrow keys.
This commit is contained in:
CJ Cenizal 2017-10-26 17:00:32 -07:00 committed by GitHub
parent bb859d93b9
commit 7a81638561
11 changed files with 172 additions and 41 deletions

View file

@ -37,7 +37,7 @@ export default class extends Component {
return (
<KuiPopover
isFocusable
ownFocus
button={button}
isOpen={this.state.isPopoverOpen}
closePopover={this.closePopover.bind(this)}

View file

@ -45,7 +45,7 @@ export default class extends Component {
return (
<div>
<KuiPopover
isFocusable
ownFocus
button={(
<KuiButton buttonType="basic" onClick={this.onButtonClick1.bind(this)}>
Popover anchored to the right.
@ -61,7 +61,7 @@ export default class extends Component {
&nbsp;
<KuiPopover
isFocusable
ownFocus
button={(
<KuiButton buttonType="basic" onClick={this.onButtonClick2.bind(this)}>
Popover anchored to the left.

View file

@ -31,7 +31,7 @@ export default class extends Component {
render() {
return (
<KuiPopover
isFocusable
ownFocus
button={(
<KuiButton buttonType="basic" onClick={this.onButtonClick.bind(this)}>
Custom class

View file

@ -3,6 +3,7 @@ import React from 'react';
import { renderToHtml } from '../../services';
import {
GuideCode,
GuideDemo,
GuidePage,
GuideSection,
@ -14,6 +15,10 @@ import Popover from './popover';
const popoverSource = require('!!raw!./popover');
const popoverHtml = renderToHtml(Popover);
import TrapFocus from './trap_focus';
const trapFocusSource = require('!!raw!./trap_focus');
const trapFocusHtml = renderToHtml(TrapFocus);
import PopoverAnchorPosition from './popover_anchor_position';
const popoverAnchorPositionSource = require('!!raw!./popover_anchor_position');
const popoverAnchorPositionHtml = renderToHtml(PopoverAnchorPosition);
@ -47,6 +52,26 @@ export default props => (
</GuideDemo>
</GuideSection>
<GuideSection
title="Trap focus"
source={[{
type: GuideSectionTypes.JS,
code: trapFocusSource,
}, {
type: GuideSectionTypes.HTML,
code: trapFocusHtml,
}]}
>
<GuideText>
If the Popover should be responsible for trapping the focus within itself (as opposed
to a child component), then you should set <GuideCode>ownFocus</GuideCode>.
</GuideText>
<GuideDemo>
<TrapFocus />
</GuideDemo>
</GuideSection>
<GuideSection
title="Popover with title"
source={[{

View file

@ -31,7 +31,7 @@ export default class extends Component {
render() {
return (
<KuiPopover
isFocusable
ownFocus
button={(
<KuiButton buttonType="basic" onClick={this.onButtonClick.bind(this)}>
Turn padding off and apply a custom class

View file

@ -41,7 +41,7 @@ export default class extends Component {
return (
<KuiPopover
isFocusable
ownFocus
button={button}
isOpen={this.state.isPopoverOpen}
closePopover={this.closePopover.bind(this)}

View file

@ -0,0 +1,75 @@
import React, {
Component,
} from 'react';
import {
KuiButton,
KuiFieldGroup,
KuiFieldGroupSection,
KuiPopover,
} from '../../../../components';
export default class extends Component {
constructor(props) {
super(props);
this.state = {
isPopoverOpen: false,
};
}
onButtonClick() {
this.setState({
isPopoverOpen: !this.state.isPopoverOpen,
});
}
closePopover() {
this.setState({
isPopoverOpen: false,
});
}
render() {
const button = (
<KuiButton buttonType="basic" onClick={this.onButtonClick.bind(this)}>
Show popover
</KuiButton>
);
return (
<KuiPopover
ownFocus
button={button}
isOpen={this.state.isPopoverOpen}
closePopover={this.closePopover.bind(this)}
>
<div className="kuiVerticalRhythmSmall">
<KuiFieldGroup>
<KuiFieldGroupSection isWide>
<div className="kuiSearchInput">
<div className="kuiSearchInput__icon kuiIcon fa-search" />
<input
className="kuiSearchInput__input"
type="text"
/>
</div>
</KuiFieldGroupSection>
<KuiFieldGroupSection>
<select className="kuiSelect">
<option>Animal</option>
<option>Mineral</option>
<option>Vegetable</option>
</select>
</KuiFieldGroupSection>
</KuiFieldGroup>
</div>
<div className="kuiVerticalRhythmSmall">
<KuiButton buttonType="primary">Save</KuiButton>
</div>
</KuiPopover>
);
}
}

View file

@ -184,7 +184,9 @@ export class KuiContextMenuPanel extends Component {
}
// Focus on the panel as a last resort.
this.panel.focus();
if (!this.panel.contains(document.activeElement)) {
this.panel.focus();
}
}
onTransitionComplete = () => {

View file

@ -42,33 +42,6 @@ exports[`KuiPopover props anchorPosition right is rendered 1`] = `
</div>
`;
exports[`KuiPopover props isFocusable defaults to false 1`] = `
<div
class="kuiPopover"
>
<button />
<div>
<div
class="kuiPanelSimple kuiPanelSimple--paddingMedium kuiPanelSimple--shadow kuiPopover__panel"
/>
</div>
</div>
`;
exports[`KuiPopover props isFocusable renders true 1`] = `
<div
class="kuiPopover"
>
<button />
<div>
<div
class="kuiPanelSimple kuiPanelSimple--paddingMedium kuiPanelSimple--shadow kuiPopover__panel"
tabindex="0"
/>
</div>
</div>
`;
exports[`KuiPopover props isOpen defaults to false 1`] = `
<div
class="kuiPopover"
@ -90,6 +63,33 @@ exports[`KuiPopover props isOpen renders true 1`] = `
</div>
`;
exports[`KuiPopover props ownFocus defaults to false 1`] = `
<div
class="kuiPopover"
>
<button />
<div>
<div
class="kuiPanelSimple kuiPanelSimple--paddingMedium kuiPanelSimple--shadow kuiPopover__panel"
/>
</div>
</div>
`;
exports[`KuiPopover props ownFocus renders true 1`] = `
<div
class="kuiPopover"
>
<button />
<div>
<div
class="kuiPanelSimple kuiPanelSimple--paddingMedium kuiPanelSimple--shadow kuiPopover__panel"
tabindex="0"
/>
</div>
</div>
`;
exports[`KuiPopover props panelClassName is rendered 1`] = `
<div
class="kuiPopover"

View file

@ -4,6 +4,7 @@ import React, {
import PropTypes from 'prop-types';
import classNames from 'classnames';
import FocusTrap from 'focus-trap-react';
import tabbable from 'tabbable';
import { cascadingMenuKeyCodes } from '../../services';
@ -37,6 +38,30 @@ export class KuiPopover extends Component {
}
};
updateFocus() {
// Wait for the DOM to update.
window.requestAnimationFrame(() => {
if (!this.panel) {
return;
}
// If we've already focused on something inside the panel, everything's fine.
if (this.panel.contains(document.activeElement)) {
return;
}
// Otherwise let's focus the first tabbable item and expedite input from the user.
const tabbableItems = tabbable(this.panel);
if (tabbableItems.length) {
tabbableItems[0].focus();
}
});
}
componentDidMount() {
this.updateFocus();
}
componentWillReceiveProps(nextProps) {
// The popover is being opened.
if (!this.props.isOpen && nextProps.isOpen) {
@ -67,12 +92,16 @@ export class KuiPopover extends Component {
}
}
componentDidUpdate() {
this.updateFocus();
}
componentWillUnmount() {
clearTimeout(this.closingTransitionTimeout);
}
panelRef = node => {
if (this.props.isFocusable) {
if (this.props.ownFocus) {
this.panel = node;
}
};
@ -82,7 +111,7 @@ export class KuiPopover extends Component {
anchorPosition,
button,
isOpen,
isFocusable,
ownFocus,
withTitle,
children,
className,
@ -110,7 +139,7 @@ export class KuiPopover extends Component {
let tabIndex;
let initialFocus;
if (isFocusable) {
if (ownFocus) {
tabIndex = '0';
initialFocus = () => this.panel;
}
@ -152,7 +181,7 @@ export class KuiPopover extends Component {
KuiPopover.propTypes = {
isOpen: PropTypes.bool,
isFocusable: PropTypes.bool,
ownFocus: PropTypes.bool,
withTitle: PropTypes.bool,
closePopover: PropTypes.func.isRequired,
button: PropTypes.node.isRequired,
@ -164,7 +193,7 @@ KuiPopover.propTypes = {
KuiPopover.defaultProps = {
isOpen: false,
isFocusable: false,
ownFocus: false,
anchorPosition: 'center',
panelPaddingSize: 'm',
};

View file

@ -135,7 +135,7 @@ describe('KuiPopover', () => {
});
});
describe('isFocusable', () => {
describe('ownFocus', () => {
test('defaults to false', () => {
const component = render(
<KuiPopover
@ -153,7 +153,7 @@ describe('KuiPopover', () => {
const component = render(
<KuiPopover
isOpen
isFocusable
ownFocus
button={<button />}
closePopover={() => {}}
/>