Skip to content

Commit

Permalink
[TreeView] disable all selection when disableSelection (mui#20146)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tonyhallett @EsoterikStare
tonyhallett authored and EsoterikStare committed Mar 30, 2020
1 parent f0b06a9 commit a5e84be
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 28 deletions.
26 changes: 10 additions & 16 deletions packages/material-ui-lab/src/TreeItem/TreeItem.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) {
isSelected,
isTabbable,
multiSelect,
selectionDisabled,
getParent,
mapFirstChar,
addNodeToNodeMap,
Expand Down Expand Up @@ -171,16 +170,14 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) {
toggleExpansion(event, nodeId);
}

if (!selectionDisabled) {
if (multiple) {
if (event.shiftKey) {
selectRange(event, { end: nodeId });
} else {
selectNode(event, nodeId, true);
}
if (multiple) {
if (event.shiftKey) {
selectRange(event, { end: nodeId });
} else {
selectNode(event, nodeId);
selectNode(event, nodeId, true);
}
} else {
selectNode(event, nodeId);
}

if (onClick) {
Expand Down Expand Up @@ -245,14 +242,12 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) {
case ' ':
if (nodeRef.current === event.currentTarget) {
if (multiSelect && event.shiftKey) {
selectRange(event, { end: nodeId });
flag = selectRange(event, { end: nodeId });
} else if (multiSelect) {
selectNode(event, nodeId, true);
flag = selectNode(event, nodeId, true);
} else {
selectNode(event, nodeId);
flag = selectNode(event, nodeId);
}

flag = true;
}
event.stopPropagation();
break;
Expand Down Expand Up @@ -310,8 +305,7 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) {
expandAllSiblings(event, nodeId);
flag = true;
} else if (multiSelect && ctrlPressed && key.toLowerCase() === 'a') {
selectAllNodes(event);
flag = true;
flag = selectAllNodes(event);
} else if (isPrintableCharacter(key)) {
flag = printableCharacter(event, key);
}
Expand Down
109 changes: 107 additions & 2 deletions packages/material-ui-lab/src/TreeItem/TreeItem.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ describe('<TreeItem />', () => {

describe('Single Selection', () => {
describe('keyboard', () => {
it('selects a node', () => {
it('should select a node when space is pressed', () => {
const { getByTestId } = render(
<TreeView>
<TreeItem nodeId="one" label="one" data-testid="one" />
Expand All @@ -711,11 +711,24 @@ describe('<TreeItem />', () => {
expect(getByTestId('one')).to.not.have.attribute('aria-selected');
fireEvent.keyDown(document.activeElement, { key: ' ' });
expect(getByTestId('one')).to.have.attribute('aria-selected', 'true');
expect(getByTestId('one')).to.have.class('Mui-selected');
});

it('should not select a node when space is pressed and disableSelection', () => {
const { getByTestId } = render(
<TreeView disableSelection>
<TreeItem nodeId="one" label="one" data-testid="one" />
</TreeView>,
);

getByTestId('one').focus();
fireEvent.keyDown(document.activeElement, { key: ' ' });
expect(getByTestId('one')).not.to.have.attribute('aria-selected');
});
});

describe('mouse', () => {
it('selects a node', () => {
it('should select a node when click', () => {
const { getByText, getByTestId } = render(
<TreeView>
<TreeItem nodeId="one" label="one" data-testid="one" />
Expand All @@ -726,6 +739,17 @@ describe('<TreeItem />', () => {
fireEvent.click(getByText('one'));
expect(getByTestId('one')).to.have.attribute('aria-selected', 'true');
});

it('should not select a node when click and disableSelection', () => {
const { getByText, getByTestId } = render(
<TreeView disableSelection>
<TreeItem nodeId="one" label="one" data-testid="one" />
</TreeView>,
);

fireEvent.click(getByText('one'));
expect(getByTestId('one')).not.to.have.attribute('aria-selected');
});
});
});

Expand Down Expand Up @@ -768,6 +792,25 @@ describe('<TreeItem />', () => {
expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(3);
});

specify('keyboard arrow does not select when selectionDisabled', () => {
const { getByTestId, getByText, container } = render(
<TreeView disableSelection multiSelect>
<TreeItem nodeId="one" label="one" data-testid="one" />
<TreeItem nodeId="two" label="two" data-testid="two" />
<TreeItem nodeId="three" label="three" data-testid="three" />
<TreeItem nodeId="four" label="four" data-testid="four" />
<TreeItem nodeId="five" label="five" data-testid="five" />
</TreeView>,
);

fireEvent.click(getByText('three'));
fireEvent.keyDown(document.activeElement, { key: 'ArrowDown', shiftKey: true });
expect(getByTestId('four')).to.have.focus;
expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(0);
fireEvent.keyDown(document.activeElement, { key: 'ArrowUp', shiftKey: true });
expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(0);
});

specify('keyboard arrow merge', () => {
const { getByTestId, getByText, container } = render(
<TreeView multiSelect defaultExpanded={['two']}>
Expand Down Expand Up @@ -872,6 +915,30 @@ describe('<TreeItem />', () => {
expect(getByTestId('nine')).to.have.attribute('aria-selected', 'false');
});

specify('keyboard home and end do not select when selectionDisabled', () => {
const { getByTestId, container } = render(
<TreeView disableSelection multiSelect defaultExpanded={['two', 'five']}>
<TreeItem nodeId="one" label="one" data-testid="one" />
<TreeItem nodeId="two" label="two" data-testid="two">
<TreeItem nodeId="three" label="three" data-testid="three" />
<TreeItem nodeId="four" label="four" data-testid="four" />
</TreeItem>
<TreeItem nodeId="five" label="five" data-testid="five">
<TreeItem nodeId="six" label="six" data-testid="six" />
<TreeItem nodeId="seven" label="seven" data-testid="seven" />
</TreeItem>
<TreeItem nodeId="eight" label="eight" data-testid="eight" />
<TreeItem nodeId="nine" label="nine" data-testid="nine" />
</TreeView>,
);

getByTestId('five').focus();
fireEvent.keyDown(document.activeElement, { key: 'End', shiftKey: true, ctrlKey: true });
expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(0);
fireEvent.keyDown(document.activeElement, { key: 'Home', shiftKey: true, ctrlKey: true });
expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(0);
});

specify('mouse', () => {
const { getByTestId, getByText } = render(
<TreeView multiSelect defaultExpanded={['two']}>
Expand Down Expand Up @@ -903,6 +970,28 @@ describe('<TreeItem />', () => {
expect(getByTestId('four')).to.have.attribute('aria-selected', 'true');
expect(getByTestId('five')).to.have.attribute('aria-selected', 'true');
});

specify('mouse does not range select when selectionDisabled', () => {
const { getByText, container } = render(
<TreeView disableSelection multiSelect defaultExpanded={['two']}>
<TreeItem nodeId="one" label="one" data-testid="one" />
<TreeItem nodeId="two" label="two" data-testid="two">
<TreeItem nodeId="three" label="three" data-testid="three" />
<TreeItem nodeId="four" label="four" data-testid="four" />
</TreeItem>
<TreeItem nodeId="five" label="five" data-testid="five">
<TreeItem nodeId="six" label="six" data-testid="six" />
<TreeItem nodeId="seven" label="seven" data-testid="seven" />
</TreeItem>
<TreeItem nodeId="eight" label="eight" data-testid="eight" />
<TreeItem nodeId="nine" label="nine" data-testid="nine" />
</TreeView>,
);

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

describe('multi selection', () => {
Expand Down Expand Up @@ -978,6 +1067,22 @@ describe('<TreeItem />', () => {
fireEvent.keyDown(document.activeElement, { key: 'a', ctrlKey: true });
expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(5);
});

specify('ctrl + a does not select all when disableSelection', () => {
const { getByTestId, container } = render(
<TreeView disableSelection multiSelect>
<TreeItem nodeId="one" label="one" data-testid="one" />
<TreeItem nodeId="two" label="two" data-testid="two" />
<TreeItem nodeId="three" label="three" data-testid="three" />
<TreeItem nodeId="four" label="four" data-testid="four" />
<TreeItem nodeId="five" label="five" data-testid="five" />
</TreeView>,
);

getByTestId('one').focus();
fireEvent.keyDown(document.activeElement, { key: 'a', ctrlKey: true });
expect(container.querySelectorAll('[aria-selected=true]').length).to.equal(0);
});
});
});

Expand Down
27 changes: 17 additions & 10 deletions packages/material-ui-lab/src/TreeView/TreeView.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,10 @@ const TreeView = React.forwardRef(function TreeView(props, ref) {
lastSelectedNode.current = id;
lastSelectionWasRange.current = false;
currentRangeSelection.current = [];

return true;
}
return false;
};

const selectRange = (event, nodes, stacked = false) => {
Expand All @@ -331,6 +334,7 @@ const TreeView = React.forwardRef(function TreeView(props, ref) {
handleRangeSelect(event, { start, end });
}
lastSelectionWasRange.current = true;
return true;
};

const rangeSelectToFirst = (event, id) => {
Expand All @@ -340,7 +344,7 @@ const TreeView = React.forwardRef(function TreeView(props, ref) {

const start = lastSelectionWasRange.current ? lastSelectedNode.current : id;

selectRange(event, {
return selectRange(event, {
start,
end: getFirstNode(),
});
Expand All @@ -353,7 +357,7 @@ const TreeView = React.forwardRef(function TreeView(props, ref) {

const start = lastSelectionWasRange.current ? lastSelectedNode.current : id;

selectRange(event, {
return selectRange(event, {
start,
end: getLastNode(),
});
Expand Down Expand Up @@ -483,6 +487,10 @@ const TreeView = React.forwardRef(function TreeView(props, ref) {
}
}, [expanded, childrenCalculated, isExpanded]);

const noopSelection = () => {
return false;
};

return (
<TreeViewContext.Provider
value={{
Expand All @@ -498,16 +506,15 @@ const TreeView = React.forwardRef(function TreeView(props, ref) {
isExpanded,
isFocused,
isSelected,
selectNode,
selectRange,
selectNextNode,
selectPreviousNode,
rangeSelectToFirst,
rangeSelectToLast,
selectAllNodes,
selectNode: disableSelection ? noopSelection : selectNode,
selectRange: disableSelection ? noopSelection : selectRange,
selectNextNode: disableSelection ? noopSelection : selectNextNode,
selectPreviousNode: disableSelection ? noopSelection : selectPreviousNode,
rangeSelectToFirst: disableSelection ? noopSelection : rangeSelectToFirst,
rangeSelectToLast: disableSelection ? noopSelection : rangeSelectToLast,
selectAllNodes: disableSelection ? noopSelection : selectAllNodes,
isTabbable,
multiSelect,
selectionDisabled: disableSelection,
getParent,
mapFirstChar,
addNodeToNodeMap,
Expand Down

0 comments on commit a5e84be

Please sign in to comment.