Skip to content

Commit

Permalink
fix(List): fix active/focus context on list
Browse files Browse the repository at this point in the history
  • Loading branch information
@aimeex3 @bfbiggs
aimeex3 authored and bfbiggs committed Jul 26, 2019
1 parent f40f0f0 commit d33929a
Show file tree
Hide file tree
Showing 8 changed files with 2,895 additions and 138 deletions.
2 changes: 2 additions & 0 deletions react/src/lib/DeviceListCall/tests/__snapshots__/index.spec.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ ShallowWrapper {
],
],
"className": "",
"focus": null,
"focusFirst": true,
"focusFirstQuery": "",
"focusQuery": "",
Expand Down Expand Up @@ -151,6 +152,7 @@ ShallowWrapper {
],
],
"className": "",
"focus": null,
"focusFirst": true,
"focusFirstQuery": "",
"focusQuery": "",
Expand Down
49 changes: 41 additions & 8 deletions react/src/lib/List/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class List extends React.Component {
last: 0,
listContext: {
active: props.active,
focus: null,
focus: (props.shouldFocusActive && props.active) || props.focus,
role: props.itemRole,
type: props.type
},
Expand All @@ -52,6 +52,10 @@ class List extends React.Component {

componentDidUpdate(prevProps, prevState) {
const { listContext } = this.state;
const { active, focus, shouldFocusActive } = this.props;
if (shouldFocusActive && (prevProps.focus !== focus || prevProps.active !== active)) {
this.setActiveAndFocus(active, focus);
}
if (!this._needsRefocus || !this.listNode) return;

if (listContext.focus && prevState.listContext.focus !== listContext.focus) {
Expand All @@ -61,10 +65,18 @@ class List extends React.Component {

determineInitialFocus = () => {
const { focusFirstQuery } = this.props;
const { listContext } = this.state;
const items = qsa(this.listNode, focusFirstQuery || `.md-list-item:not(.disabled):not(:disabled):not(.md-list-item--read-only)`);

this._needsRefocus = true;
items.length && this.getNextFocusedChild(items, items[0], 0);
let focus = listContext.focus;
if (items.length) {
if (!focus) {
focus = this.getNextFocusedChild(items, items[0], 0);
}
if (focus) {
this.listNode.querySelector(`[data-md-event-key="${focus}"]`).focus();
}
}
}

getIncludesFirstCharacter = (str, char) =>
Expand Down Expand Up @@ -92,13 +104,15 @@ class List extends React.Component {
} else return possibleIndex;
};

return listContext.focus !== this.getValue(items, getIndex(), 'event')
listContext.focus !== this.getValue(items, getIndex(), 'event')
&& this.setState({
listContext: {
...listContext,
focus: this.getValue(items, getIndex(), 'event'),
}
});

return this.getValue(items, getIndex(), 'event');
}

getValue = (arr, index, attribute) => (
Expand All @@ -112,9 +126,9 @@ class List extends React.Component {

const defaultItems = qsa(this.listNode, `.md-list-item:not(.disabled):not(:disabled):not(.md-list-item--read-only)`);
const customItems = focusQuery && qsa(this.listNode, focusQuery) || [];
return customItems.length
? customItems.filter(item => customItems.indexOf(item) >= 0)

return customItems.length
? customItems.filter(item => customItems.indexOf(item) >= 0)
: defaultItems;
}

Expand Down Expand Up @@ -247,6 +261,17 @@ class List extends React.Component {
}));
}

setActiveAndFocus = (active, focus) => {
this._needsRefocus = false;
this.setState(state => ({
listContext: {
...state.listContext,
active: active,
focus: (state.shouldFocusActive && active) || focus,
}
}));
}

setFocusByFirstCharacter = (char, focusIdx, items, length) => {
const { listContext } = this.state;
const newIndex = items
Expand Down Expand Up @@ -278,10 +303,15 @@ class List extends React.Component {
}

setFocusToActive() {
let focus = this.state.listContext.active;
if (!focus) {
const items = this.getFocusableItems();
focus = this.getValue(items, 0, 'event');
}
this.setState({
listContext: {
...this.state.listContext,
focus: this.state.listContext.active,
focus,
}
});
}
Expand Down Expand Up @@ -374,6 +404,8 @@ List.propTypes = {
children: PropTypes.node,
/** @prop Optional css class string | '' */
className: PropTypes.string,
/** @prop Optional focus prop to pass focus prop to children | null */
focus: PropTypes.string,
/** @prop Sets first List item to have focus | true */
focusFirst: PropTypes.bool,
/** @prop Queries children to find matching item to have focus | '' */
Expand Down Expand Up @@ -408,6 +440,7 @@ List.defaultProps = {
className: '',
id: null,
itemRole: 'listitem',
focus: null,
focusFirst: true,
focusFirstQuery: '',
focusQuery: '',
Expand Down
7 changes: 7 additions & 0 deletions react/src/lib/List/tests/__snapshots__/index.spec.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ ShallowWrapper {
Symbol(enzyme.__unrendered__): <List
active={null}
className=""
focus={null}
focusFirst={true}
focusFirstQuery=""
focusQuery=""
Expand Down Expand Up @@ -47,6 +48,7 @@ ShallowWrapper {
<div
aria-activedescendant={null}
className="md-list md-list--vertical"
focus={null}
id="test"
onSelect={null}
role="list"
Expand All @@ -66,6 +68,7 @@ ShallowWrapper {
"children": <div
aria-activedescendant={null}
className="md-list md-list--vertical"
focus={null}
id="test"
onSelect={null}
role="list"
Expand All @@ -86,6 +89,7 @@ ShallowWrapper {
"aria-activedescendant": null,
"children": null,
"className": "md-list md-list--vertical",
"focus": null,
"id": "test",
"onSelect": null,
"role": "list",
Expand Down Expand Up @@ -151,6 +155,7 @@ ShallowWrapper {
<div
aria-activedescendant={null}
className="md-list md-list--vertical"
focus={null}
id="test"
onSelect={null}
role="list"
Expand All @@ -170,6 +175,7 @@ ShallowWrapper {
"children": <div
aria-activedescendant={null}
className="md-list md-list--vertical"
focus={null}
id="test"
onSelect={null}
role="list"
Expand All @@ -190,6 +196,7 @@ ShallowWrapper {
"aria-activedescendant": null,
"children": null,
"className": "md-list md-list--vertical",
"focus": null,
"id": "test",
"onSelect": null,
"role": "list",
Expand Down
40 changes: 40 additions & 0 deletions react/src/lib/List/tests/index.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ import {
} from '@momentum-ui/react';

describe('tests for <List />', () => {
beforeEach(() => {
document.activeElement.blur();
});

it('should match SnapShot', () => {
const container = shallow(<List id="test" />);

Expand Down Expand Up @@ -185,6 +189,42 @@ describe('tests for <List />', () => {
expect(container.state().listContext.focus).toEqual('test-list-3');
});

it('should initially set focus and active values', () => {
const container = mount(
<List active='test-list-2' shouldFocusActive focusFirst={false}>
<ListItem className='firstIndex' label="test" id='test-list-1' link='javscript:void(0)' />
<ListItem className='secondIndex' label="test" id='test-list-2' link='javscript:void(0)' />
<ListItem className='thirdIndex' label="test" id='test-list-3' link='javscript:void(0)' />
</List>
);

expect(container.state().listContext.active).toEqual('test-list-2');
expect(container.state().listContext.focus).toEqual('test-list-2');
expect(container.find('.md-list-item').at(0).props().tabIndex).toEqual(-1);
expect(container.find('.md-list-item').at(1).props().tabIndex).toEqual(0);
expect(container.find('.md-list-item').at(2).props().tabIndex).toEqual(-1);
expect(document.hasFocus()).toEqual(false);
expect(document.activeElement.className).toBe('');
});

it('should initially focus on active item', () => {
const container = mount(
<List active='test-list-2' shouldFocusActive focusFirst={true}>
<ListItem className='firstIndex' label="test" id='test-list-1' link='javscript:void(0)' />
<ListItem className='secondIndex' label="test" id='test-list-2' link='javscript:void(0)' />
<ListItem className='thirdIndex' label="test" id='test-list-3' link='javscript:void(0)' />
</List>
);

expect(container.state().listContext.active).toEqual('test-list-2');
expect(container.state().listContext.focus).toEqual('test-list-2');
expect(container.find('.md-list-item').at(0).props().tabIndex).toEqual(-1);
expect(container.find('.md-list-item').at(1).props().tabIndex).toEqual(0);
expect(container.find('.md-list-item').at(2).props().tabIndex).toEqual(-1);
expect(document.hasFocus()).toEqual(true);
expect(document.activeElement.className).toContain('secondIndex');
});

it('should not track active', () => {
const container = mount(
<List trackActive={false}>
Expand Down
5 changes: 2 additions & 3 deletions react/src/lib/ListItem/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import ReactDOM from 'react-dom';
import PropTypes from 'prop-types';
import omit from 'lodash/omit';
import { UIDConsumer } from 'react-uid';
import SelectableContext, { makeEventKey, makeKeyboardKey } from '../SelectableContext';
import SelectableContext, { makeKeyboardKey } from '../SelectableContext';
import ListContext from '../ListContext';
import mapContextToProps from 'react-context-toolbox/mapContextToProps';

Expand Down Expand Up @@ -129,7 +129,6 @@ class ListItem extends React.Component {
...props
} = this.props;

const navKey = makeEventKey(eventKey);
const keyboardNavKey = makeKeyboardKey(keyboardKey || title || label);

const otherProps = omit({...props}, [
Expand Down Expand Up @@ -198,7 +197,7 @@ class ListItem extends React.Component {
let contextProps = {};

contextProps.id = this.props.id || id;
contextProps.uniqueKey = navKey || contextProps.id;
contextProps.uniqueKey = eventKey || contextProps.id;
contextProps.type = type || (listContext && listContext.type);
contextProps.focus = focus || (listContext && listContext.focus === contextProps.uniqueKey);
contextProps.active = active || (listContext && listContext.active === contextProps.uniqueKey);
Expand Down
8 changes: 8 additions & 0 deletions react/src/lib/SidebarNav/tests/__snapshots__/index.spec.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ ShallowWrapper {
<List
active={null}
className="md-sidebar-nav__group md-sidebar-nav__group--primary"
focus={null}
focusFirst={true}
focusFirstQuery=""
focusQuery="
Expand Down Expand Up @@ -72,6 +73,7 @@ ShallowWrapper {
<List
active={null}
className="md-sidebar-nav__group md-sidebar-nav__group--primary"
focus={null}
focusFirst={true}
focusFirstQuery=""
focusQuery="
Expand Down Expand Up @@ -108,6 +110,7 @@ ShallowWrapper {
<List
active={null}
className="md-sidebar-nav__group md-sidebar-nav__group--primary"
focus={null}
focusFirst={true}
focusFirstQuery=""
focusQuery="
Expand Down Expand Up @@ -142,6 +145,7 @@ ShallowWrapper {
"active": null,
"children": null,
"className": "md-sidebar-nav__group md-sidebar-nav__group--primary",
"focus": null,
"focusFirst": true,
"focusFirstQuery": "",
"focusQuery": "
Expand Down Expand Up @@ -209,6 +213,7 @@ ShallowWrapper {
<List
active={null}
className="md-sidebar-nav__group md-sidebar-nav__group--primary"
focus={null}
focusFirst={true}
focusFirstQuery=""
focusQuery="
Expand Down Expand Up @@ -244,6 +249,7 @@ ShallowWrapper {
<List
active={null}
className="md-sidebar-nav__group md-sidebar-nav__group--primary"
focus={null}
focusFirst={true}
focusFirstQuery=""
focusQuery="
Expand Down Expand Up @@ -280,6 +286,7 @@ ShallowWrapper {
<List
active={null}
className="md-sidebar-nav__group md-sidebar-nav__group--primary"
focus={null}
focusFirst={true}
focusFirstQuery=""
focusQuery="
Expand Down Expand Up @@ -314,6 +321,7 @@ ShallowWrapper {
"active": null,
"children": null,
"className": "md-sidebar-nav__group md-sidebar-nav__group--primary",
"focus": null,
"focusFirst": true,
"focusFirstQuery": "",
"focusQuery": "
Expand Down
4 changes: 4 additions & 0 deletions react/src/lib/TopbarNav/tests/__snapshots__/index.spec.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ ShallowWrapper {
"children": <List
active={null}
className=""
focus={null}
focusFirst={true}
focusFirstQuery=""
focusQuery=""
Expand All @@ -49,6 +50,7 @@ ShallowWrapper {
"active": null,
"children": null,
"className": "",
"focus": null,
"focusFirst": true,
"focusFirstQuery": "",
"focusQuery": "",
Expand Down Expand Up @@ -78,6 +80,7 @@ ShallowWrapper {
"children": <List
active={null}
className=""
focus={null}
focusFirst={true}
focusFirstQuery=""
focusQuery=""
Expand All @@ -103,6 +106,7 @@ ShallowWrapper {
"active": null,
"children": null,
"className": "",
"focus": null,
"focusFirst": true,
"focusFirstQuery": "",
"focusQuery": "",
Expand Down
Loading

0 comments on commit d33929a

Please sign in to comment.