From a5e84be058fa3d9a64b975f7c65c9eaba09a75da Mon Sep 17 00:00:00 2001
From: Tony Hallett <tonyhallett74@gmail.com>
Date: Wed, 25 Mar 2020 12:23:46 +0000
Subject: [PATCH] [TreeView] disable all selection when disableSelection
 (#20146)

---
 .../material-ui-lab/src/TreeItem/TreeItem.js  |  26 ++---
 .../src/TreeItem/TreeItem.test.js             | 109 +++++++++++++++++-
 .../material-ui-lab/src/TreeView/TreeView.js  |  27 +++--
 3 files changed, 134 insertions(+), 28 deletions(-)

diff --git a/packages/material-ui-lab/src/TreeItem/TreeItem.js b/packages/material-ui-lab/src/TreeItem/TreeItem.js
index d6f8a835eed085..785cc8e27c5b8c 100644
--- a/packages/material-ui-lab/src/TreeItem/TreeItem.js
+++ b/packages/material-ui-lab/src/TreeItem/TreeItem.js
@@ -122,7 +122,6 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) {
     isSelected,
     isTabbable,
     multiSelect,
-    selectionDisabled,
     getParent,
     mapFirstChar,
     addNodeToNodeMap,
@@ -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) {
@@ -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;
@@ -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);
         }
diff --git a/packages/material-ui-lab/src/TreeItem/TreeItem.test.js b/packages/material-ui-lab/src/TreeItem/TreeItem.test.js
index a9c2dbe1db6e6e..b759a917f5a588 100644
--- a/packages/material-ui-lab/src/TreeItem/TreeItem.test.js
+++ b/packages/material-ui-lab/src/TreeItem/TreeItem.test.js
@@ -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" />
@@ -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" />
@@ -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');
+        });
       });
     });
 
@@ -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']}>
@@ -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']}>
@@ -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', () => {
@@ -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);
+      });
     });
   });
 
diff --git a/packages/material-ui-lab/src/TreeView/TreeView.js b/packages/material-ui-lab/src/TreeView/TreeView.js
index b60bd1801900f5..7473a3b2e7ed91 100644
--- a/packages/material-ui-lab/src/TreeView/TreeView.js
+++ b/packages/material-ui-lab/src/TreeView/TreeView.js
@@ -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) => {
@@ -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) => {
@@ -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(),
     });
@@ -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(),
     });
@@ -483,6 +487,10 @@ const TreeView = React.forwardRef(function TreeView(props, ref) {
     }
   }, [expanded, childrenCalculated, isExpanded]);
 
+  const noopSelection = () => {
+    return false;
+  };
+
   return (
     <TreeViewContext.Provider
       value={{
@@ -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,