Skip to content

Commit

Permalink
fix(Tabs): prevent update selecton upon arrow keys (#5659)
Browse files Browse the repository at this point in the history
This change introduces `selectionMode="manual"` prop to
`<ContentSwitcher>` and `<Tabs>` that makes left/right arrow keys in
those components change only the focused item, not the selection. User
can use space/enter keys to update the selection.

Fixes #4870.
Fixes #5657.
  • Loading branch information
asudoh authored Mar 27, 2020
1 parent 713ed57 commit 19d8aeb
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 42 deletions.
20 changes: 20 additions & 0 deletions packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,7 @@ Map {
"ContentSwitcher" => Object {
"defaultProps": Object {
"selectedIndex": 0,
"selectionMode": "automatic",
},
"propTypes": Object {
"children": Object {
Expand All @@ -663,6 +664,15 @@ Map {
"selectedIndex": Object {
"type": "number",
},
"selectionMode": Object {
"args": Array [
Array [
"automatic",
"manual",
],
],
"type": "oneOf",
},
},
},
"Copy" => Object {
Expand Down Expand Up @@ -4679,6 +4689,7 @@ Map {
"iconDescription": "show menu options",
"role": "navigation",
"selected": 0,
"selectionMode": "automatic",
"triggerHref": "#",
"type": "default",
},
Expand Down Expand Up @@ -4715,6 +4726,15 @@ Map {
"selected": Object {
"type": "number",
},
"selectionMode": Object {
"args": Array [
Array [
"automatic",
"manual",
],
],
"type": "oneOf",
},
"tabContentClassName": Object {
"type": "string",
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,22 @@
import React from 'react';
import { storiesOf } from '@storybook/react';
import { action } from '@storybook/addon-actions';
import { withKnobs, boolean } from '@storybook/addon-knobs';
import { withKnobs, boolean, select } from '@storybook/addon-knobs';
import ContentSwitcher from '../ContentSwitcher';
import Switch from '../Switch';

const selectionModes = {
'Change selection automatically upon focus (automatic)': 'automatic',
'Change selection on explicit gesture (manual)': 'manual',
};

const props = {
contentSwitcher: () => ({
selectionMode: select(
'Selection mode (selectionMode)',
selectionModes,
'automatic'
),
onChange: action('onChange'),
}),
switch: () => ({
Expand Down
38 changes: 23 additions & 15 deletions packages/react/src/components/ContentSwitcher/ContentSwitcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,16 @@ export default class ContentSwitcher extends React.Component {
* Specify a selected index for the initially selected content
*/
selectedIndex: PropTypes.number,

/**
* Choose whether or not to automatically change selection on focus
*/
selectionMode: PropTypes.oneOf(['automatic', 'manual']),
};

static defaultProps = {
selectedIndex: 0,
selectionMode: 'automatic',
};

static getDerivedStateFromProps({ selectedIndex }, state) {
Expand All @@ -66,28 +72,30 @@ export default class ContentSwitcher extends React.Component {
};

handleChildChange = data => {
const { selectionMode } = this.props;
// the currently selected child index
const { selectedIndex } = this.state;
// the newly selected child index
const { index } = data;
const { key } = data;

if (matches(data, [keys.ArrowRight, keys.ArrowLeft])) {
const nextIndex = getNextIndex(
key,
selectedIndex,
this.props.children.length
);
this.setState(
{
selectedIndex: nextIndex,
},
() => {
const switchRef = this._switchRefs[nextIndex];
switchRef && switchRef.focus();
this.props.onChange(data);
}
);
const nextIndex = getNextIndex(key, index, this.props.children.length);
if (selectionMode === 'manual') {
const switchRef = this._switchRefs[nextIndex];
switchRef && switchRef.focus();
} else {
this.setState(
{
selectedIndex: nextIndex,
},
() => {
const switchRef = this._switchRefs[nextIndex];
switchRef && switchRef.focus();
this.props.onChange(data);
}
);
}
} else {
if (selectedIndex !== index) {
this.setState({ selectedIndex: index }, () => {
Expand Down
18 changes: 17 additions & 1 deletion packages/react/src/components/Tabs/Tabs-story.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@
import React from 'react';
import { storiesOf } from '@storybook/react';
import { action } from '@storybook/addon-actions';
import { withKnobs, boolean, number, text } from '@storybook/addon-knobs';
import {
withKnobs,
boolean,
number,
select,
text,
} from '@storybook/addon-knobs';
import { settings } from 'carbon-components';
import classNames from 'classnames';
import './Tabs-story.scss';
Expand All @@ -17,6 +23,11 @@ import Tabs from '../Tabs';
import Tab from '../Tab';
import TabsSkeleton from '../Tabs/Tabs.Skeleton';

const selectionModes = {
'Change selection automatically upon focus (automatic)': 'automatic',
'Change selection on explicit gesture (manual)': 'manual',
};

const { prefix } = settings;
const props = {
tabs: () => ({
Expand All @@ -39,6 +50,11 @@ const props = {
'The className for the child `<TabContent>` components',
'tab-content'
),
selectionMode: select(
'Selection mode (selectionMode)',
selectionModes,
'automatic'
),
}),
tab: () => ({
disabled: boolean('Disabled (disabled in <Tab>)', false),
Expand Down
78 changes: 54 additions & 24 deletions packages/react/src/components/Tabs/Tabs-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,40 +209,56 @@ describe('Tabs', () => {
});

describe('keydown', () => {
const wrapper = mount(
<Tabs selected={0}>
<Tab label="firstTab" className="firstTab">
content
</Tab>
<Tab label="lastTab" className="lastTab">
content
</Tab>
</Tabs>
);

const firstTab = wrapper.find('.firstTab').last();
const lastTab = wrapper.find('.lastTab').last();
const leftKey = 37;
const rightKey = 39;
const spaceKey = 32;
const enterKey = 13;

let wrapper;
let firstTab;
let lastTab;
let anchorInFirstTab;
let anchorInLastTab;
let spyFocusAnchorInFirstTab;
let spyFocusAnchorInLastTab;

describe('state: selected', () => {
beforeEach(() => {
wrapper = mount(
<Tabs selected={0}>
<Tab label="firstTab" className="firstTab">
content
</Tab>
<Tab label="lastTab" className="lastTab">
content
</Tab>
</Tabs>
);
firstTab = wrapper.find('.firstTab').last();
lastTab = wrapper.find('.lastTab').last();
anchorInFirstTab = firstTab.find('a').getDOMNode();
anchorInLastTab = lastTab.find('a').getDOMNode();
});

it('updates selected state when pressing arrow keys', () => {
spyFocusAnchorInFirstTab = jest.spyOn(anchorInFirstTab, 'focus');
spyFocusAnchorInLastTab = jest.spyOn(anchorInLastTab, 'focus');
firstTab.simulate('keydown', { which: rightKey });
expect(wrapper.state().selected).toEqual(1);
expect(spyFocusAnchorInLastTab).toHaveBeenCalled();
lastTab.simulate('keydown', { which: leftKey });
expect(wrapper.state().selected).toEqual(0);
expect(spyFocusAnchorInFirstTab).toHaveBeenCalled();
});

it('loops focus and selected state from lastTab to firstTab', () => {
spyFocusAnchorInFirstTab = jest.spyOn(anchorInFirstTab, 'focus');
lastTab.simulate('keydown', { which: rightKey });
expect(wrapper.state().selected).toEqual(0);
expect(spyFocusAnchorInFirstTab).toHaveBeenCalled();
});

it('loops focus and selected state from firstTab to lastTab', () => {
spyFocusAnchorInLastTab = jest.spyOn(anchorInLastTab, 'focus');
firstTab.simulate('keydown', { which: leftKey });
expect(wrapper.state().selected).toEqual(1);
expect(spyFocusAnchorInLastTab).toHaveBeenCalled();
});

it('updates selected state when pressing space or enter key', () => {
Expand All @@ -254,9 +270,6 @@ describe('Tabs', () => {
});

describe('ignore disabled child tab', () => {
let wrapper;
let firstTab;
let lastTab;
beforeEach(() => {
wrapper = mount(
<Tabs>
Expand All @@ -273,23 +286,29 @@ describe('Tabs', () => {
);
firstTab = wrapper.find('.firstTab').last();
lastTab = wrapper.find('.lastTab').last();
anchorInFirstTab = firstTab.find('a').getDOMNode();
anchorInLastTab = lastTab.find('a').getDOMNode();
});
it('updates selected state when pressing arrow keys', () => {
spyFocusAnchorInFirstTab = jest.spyOn(anchorInFirstTab, 'focus');
spyFocusAnchorInLastTab = jest.spyOn(anchorInLastTab, 'focus');
firstTab.simulate('keydown', { which: rightKey });
expect(wrapper.state().selected).toEqual(2);
expect(spyFocusAnchorInLastTab).toHaveBeenCalled();
lastTab.simulate('keydown', { which: leftKey });
expect(wrapper.state().selected).toEqual(0);
expect(spyFocusAnchorInFirstTab).toHaveBeenCalled();
});

it('loops focus and selected state from lastTab to firstTab', () => {
spyFocusAnchorInFirstTab = jest.spyOn(anchorInFirstTab, 'focus');
wrapper.setState({ selected: 2 });
lastTab.simulate('keydown', { which: rightKey });
expect(wrapper.state().selected).toEqual(0);
expect(spyFocusAnchorInFirstTab).toHaveBeenCalled();
});

it('loops focus and selected state from firstTab to lastTab', () => {
spyFocusAnchorInLastTab = jest.spyOn(anchorInLastTab, 'focus');
firstTab.simulate('keydown', { which: leftKey });
expect(wrapper.state().selected).toEqual(2);
expect(spyFocusAnchorInLastTab).toHaveBeenCalled();
});

it('updates selected state when pressing space or enter key', () => {
Expand All @@ -299,6 +318,17 @@ describe('Tabs', () => {
expect(wrapper.state().selected).toEqual(2);
});
});

afterEach(() => {
if (spyFocusAnchorInLastTab) {
spyFocusAnchorInLastTab.mockRestore();
spyFocusAnchorInLastTab = null;
}
if (spyFocusAnchorInFirstTab) {
spyFocusAnchorInFirstTab.mockRestore();
spyFocusAnchorInFirstTab = null;
}
});
});
});

Expand Down
10 changes: 9 additions & 1 deletion packages/react/src/components/Tabs/Tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ export default class Tabs extends React.Component {
* Provide a className that is applied to the <TabContent> components
*/
tabContentClassName: PropTypes.string,

/**
* Choose whether or not to automatically change selection on focus
*/
selectionMode: PropTypes.oneOf(['automatic', 'manual']),
};

static defaultProps = {
Expand All @@ -98,6 +103,7 @@ export default class Tabs extends React.Component {
triggerHref: '#',
selected: 0,
ariaLabel: 'listbox',
selectionMode: 'automatic',
};

state = {
Expand Down Expand Up @@ -184,7 +190,9 @@ export default class Tabs extends React.Component {
const tab = this.getTabAt(nextIndex);
if (tab && matches(evt, [keys.ArrowLeft, keys.ArrowRight])) {
evt.preventDefault();
this.selectTabAt(nextIndex, onSelectionChange);
if (this.props.selectionMode !== 'manual') {
this.selectTabAt(nextIndex, onSelectionChange);
}
if (tab.tabAnchor) {
tab.tabAnchor.focus();
}
Expand Down

0 comments on commit 19d8aeb

Please sign in to comment.