Skip to content

Commit

Permalink
Use #20147 and modify slightly
Browse files Browse the repository at this point in the history
  • Loading branch information
joshwooding committed Jun 27, 2020
1 parent 24376ab commit 154c8f1
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 22 deletions.
4 changes: 3 additions & 1 deletion packages/material-ui-lab/src/TreeItem/TreeItem.js
Expand Up @@ -342,11 +342,13 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) {
};

React.useEffect(() => {
// On the first render a node's index will be -1. We want to wait for the real index.
if (registerNode && unregisterNode && index !== -1) {
registerNode({
id: nodeId,
index,
parentId,
expandable,
});

return () => {
Expand All @@ -355,7 +357,7 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) {
}

return undefined;
}, [registerNode, unregisterNode, parentId, index, nodeId]);
}, [registerNode, unregisterNode, parentId, index, nodeId, expandable]);

React.useEffect(() => {
if (mapFirstChar && label) {
Expand Down
35 changes: 19 additions & 16 deletions packages/material-ui-lab/src/TreeItem/TreeItem.test.js
Expand Up @@ -837,8 +837,9 @@ describe('<TreeItem />', () => {

describe('asterisk key interaction', () => {
it('expands all siblings that are at the same level as the current node', () => {
const toggleSpy = spy();
const { getByTestId } = render(
<TreeView>
<TreeView onNodeToggle={toggleSpy}>
<TreeItem nodeId="one" label="one" data-testid="one">
<TreeItem nodeId="two" label="two" data-testid="two" />
</TreeItem>
Expand All @@ -862,6 +863,8 @@ describe('<TreeItem />', () => {

fireEvent.keyDown(getByTestId('one'), { key: '*' });

expect(toggleSpy.args[0][1]).to.have.length(3);

expect(getByTestId('one')).to.have.attribute('aria-expanded', 'true');
expect(getByTestId('three')).to.have.attribute('aria-expanded', 'true');
expect(getByTestId('five')).to.have.attribute('aria-expanded', 'true');
Expand Down Expand Up @@ -989,27 +992,27 @@ describe('<TreeItem />', () => {
fireEvent.keyDown(getByTestId('three'), { key: 'ArrowDown', shiftKey: true });

expect(getByTestId('four')).toHaveFocus();
expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(2);
expect(container.querySelectorAll('[aria-selected=true]')).to.have.length(2);

fireEvent.keyDown(getByTestId('four'), { key: 'ArrowDown', shiftKey: true });

expect(getByTestId('three')).to.have.attribute('aria-selected', 'true');
expect(getByTestId('four')).to.have.attribute('aria-selected', 'true');
expect(getByTestId('five')).to.have.attribute('aria-selected', 'true');
expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(3);
expect(container.querySelectorAll('[aria-selected=true]')).to.have.length(3);

fireEvent.keyDown(getByTestId('five'), { key: 'ArrowUp', shiftKey: true });

expect(getByTestId('four')).toHaveFocus();
expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(2);
expect(container.querySelectorAll('[aria-selected=true]')).to.have.length(2);

fireEvent.keyDown(getByTestId('four'), { key: 'ArrowUp', shiftKey: true });

expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(1);
expect(container.querySelectorAll('[aria-selected=true]')).to.have.length(1);

fireEvent.keyDown(getByTestId('three'), { key: 'ArrowUp', shiftKey: true });

expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(2);
expect(container.querySelectorAll('[aria-selected=true]')).to.have.length(2);

fireEvent.keyDown(getByTestId('two'), { key: 'ArrowUp', shiftKey: true });

Expand All @@ -1018,7 +1021,7 @@ describe('<TreeItem />', () => {
expect(getByTestId('three')).to.have.attribute('aria-selected', 'true');
expect(getByTestId('four')).to.have.attribute('aria-selected', 'false');
expect(getByTestId('five')).to.have.attribute('aria-selected', 'false');
expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(3);
expect(container.querySelectorAll('[aria-selected=true]')).to.have.length(3);
});

specify('keyboard arrow does not select when selectionDisabled', () => {
Expand All @@ -1036,11 +1039,11 @@ describe('<TreeItem />', () => {
fireEvent.keyDown(getByTestId('three'), { key: 'ArrowDown', shiftKey: true });

expect(getByTestId('four')).toHaveFocus();
expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(0);
expect(container.querySelectorAll('[aria-selected=true]')).to.have.length(0);

fireEvent.keyDown(getByTestId('four'), { key: 'ArrowUp', shiftKey: true });

expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(0);
expect(container.querySelectorAll('[aria-selected=true]')).to.have.length(0);
});

specify('keyboard arrow merge', () => {
Expand All @@ -1066,12 +1069,12 @@ describe('<TreeItem />', () => {
fireEvent.keyDown(getByTestId('four'), { key: 'ArrowUp', shiftKey: true });
fireEvent.keyDown(getByTestId('three'), { key: 'ArrowUp', shiftKey: true });

expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(5);
expect(container.querySelectorAll('[aria-selected=true]')).to.have.length(5);

fireEvent.keyDown(getByTestId('two'), { key: 'ArrowDown', shiftKey: true });
fireEvent.keyDown(getByTestId('three'), { key: 'ArrowDown', shiftKey: true });

expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(3);
expect(container.querySelectorAll('[aria-selected=true]')).to.have.length(3);
});

specify('keyboard space', () => {
Expand Down Expand Up @@ -1178,11 +1181,11 @@ describe('<TreeItem />', () => {
getByTestId('five').focus();
fireEvent.keyDown(getByTestId('five'), { key: 'End', shiftKey: true, ctrlKey: true });

expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(0);
expect(container.querySelectorAll('[aria-selected=true]')).to.have.length(0);

fireEvent.keyDown(getByTestId('nine'), { key: 'Home', shiftKey: true, ctrlKey: true });

expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(0);
expect(container.querySelectorAll('[aria-selected=true]')).to.have.length(0);
});

specify('mouse', () => {
Expand Down Expand Up @@ -1236,7 +1239,7 @@ describe('<TreeItem />', () => {

fireEvent.click(getByText('five'));
fireEvent.click(getByText('nine'), { shiftKey: true });
expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(0);
expect(container.querySelectorAll('[aria-selected=true]')).to.have.length(0);
});
});

Expand Down Expand Up @@ -1317,7 +1320,7 @@ describe('<TreeItem />', () => {
getByTestId('one').focus();
fireEvent.keyDown(getByTestId('one'), { key: 'a', ctrlKey: true });

expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(5);
expect(container.querySelectorAll('[aria-selected=true]')).to.have.length(5);
});

specify('ctrl + a does not select all when disableSelection', () => {
Expand All @@ -1334,7 +1337,7 @@ describe('<TreeItem />', () => {
getByTestId('one').focus();
fireEvent.keyDown(getByTestId('one'), { key: 'a', ctrlKey: true });

expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(0);
expect(container.querySelectorAll('[aria-selected=true]')).to.have.length(0);
});
});
});
Expand Down
12 changes: 7 additions & 5 deletions packages/material-ui-lab/src/TreeView/TreeView.js
Expand Up @@ -94,6 +94,8 @@ const TreeView = React.forwardRef(function TreeView(props, ref) {
[expanded],
);

const isExpandable = React.useCallback((id) => nodeMap.current[id]?.expandable, []);

const isSelected = React.useCallback(
(id) => (Array.isArray(selected) ? selected.indexOf(id) !== -1 : selected === id),
[selected],
Expand Down Expand Up @@ -295,7 +297,7 @@ const TreeView = React.forwardRef(function TreeView(props, ref) {
newExpanded = expanded.filter((id) => id !== value);
setTabbable((oldTabbable) => {
const map = nodeMap.current[oldTabbable];
if (oldTabbable && map && map.parentId === value) {
if (oldTabbable && map?.parentId === value) {
return value;
}
return oldTabbable;
Expand All @@ -315,7 +317,7 @@ const TreeView = React.forwardRef(function TreeView(props, ref) {
const map = nodeMap.current[id];
const siblings = getChildren(map.parentId);

const diff = siblings.filter((child) => !isExpanded(child));
const diff = siblings.filter((child) => isExpandable(child) && !isExpanded(child));

const newExpanded = expanded.concat(diff);

Expand Down Expand Up @@ -495,9 +497,9 @@ const TreeView = React.forwardRef(function TreeView(props, ref) {
* Mapping Helpers
*/
const registerNode = React.useCallback((node) => {
const { id, index, parentId } = node;
const { id, index, parentId, expandable } = node;

nodeMap.current[id] = { id, index, parentId };
nodeMap.current[id] = { id, index, parentId, expandable };
}, []);

const getNodesToRemove = React.useCallback((id) => {
Expand Down Expand Up @@ -555,7 +557,7 @@ const TreeView = React.forwardRef(function TreeView(props, ref) {
React.useEffect(() => {
setTabbable((oldTabbable) => {
if (!oldTabbable) {
return descendants[0] ? descendants[0].id : null;
return descendants[0]?.id;
}

return oldTabbable;
Expand Down
4 changes: 4 additions & 0 deletions packages/material-ui-lab/src/TreeView/descendants.js
@@ -1,6 +1,10 @@
import * as React from 'react';
import PropTypes from 'prop-types';

/** Credit: https://github.com/reach/reach-ui/blob/86a046f54d53b6420e392b3fa56dd991d9d4e458/packages/descendants/README.md
* Modified slightly to suit our purposes.
*/

// To replace with .findIndex() once we stop IE 11 support.
function findIndex(array, comp) {
for (let i = 0; i < array.length; i += 1) {
Expand Down

0 comments on commit 154c8f1

Please sign in to comment.