Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TreeView] Disable all selection when disableSelection #20146

Merged
merged 3 commits into from
Mar 25, 2020
Merged

[TreeView] Disable all selection when disableSelection #20146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
031ef09
[TreeView] disable all selection when disableSelection
tonyhallett Mar 16, 2020
72e2846
Merge remote-tracking branch 'upstream/master' into disableSelection-…
tonyhallett Mar 23, 2020
c52868c
manage selection when disabled with the TreeView
tonyhallett Mar 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 10 additions & 16 deletions packages/material-ui-lab/src/TreeItem/TreeItem.js
View file
Open in desktop
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
Open in desktop
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');
tonyhallett marked this conversation as resolved.
Show resolved Hide resolved
});

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
Open in desktop
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