Seb feedback

mui · Jul 27, 2020 · 6c45699 · 6c45699
1 parent bcf5789
commit 6c45699
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 22 deletions.
diff --git a/docs/src/pages/components/tree-view/tree-view.md b/docs/src/pages/components/tree-view/tree-view.md
@@ -65,21 +65,21 @@ The behaviour of disabled tree items depends on the `disabledItemsFocusable` pro
 
 If it is false:
 
-- Mouse clicks will not focus disabled items.
 - Arrow keys will not focus disabled items and, the next non-disabled item will be focused.
 - Typing the first character of a disabled item's label will not focus the item.
 - Mouse or keyboard interaction will not expand/collapse disabled items.
 - Mouse or keyboard interaction will not select disabled items.
 - Shift + arrow keys will skip disabled items and, the next non-disabled item will be selected.
+- Programmatic focus will not focus disabled items.
 
 If it is true:
 
-- Mouse clicks will not focus disabled items.
 - Arrow keys will focus disabled items.
 - Typing the first character of a disabled item's label will focus the item.
 - Mouse or keyboard interaction will not expand/collapse disabled items.
 - Mouse or keyboard interaction will not select disabled items.
 - Shift + arrow keys will not skip disabled items but, the disabled item will not be selected.
+- Programmatic focus will focus disabled items.
 
 ## Accessibility
 

diff --git a/packages/material-ui-lab/src/TreeItem/TreeItem.js b/packages/material-ui-lab/src/TreeItem/TreeItem.js
@@ -208,10 +208,8 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) {
     }
   };
 
-  const wasClick = React.useRef(false);
   const handleMouseDown = (event) => {
-    wasClick.current = true;
-    if (event.shiftKey || event.ctrlKey || event.metaKey) {
+    if (event.shiftKey || event.ctrlKey || event.metaKey || disabled) {
       // Prevent text selection
       event.preventDefault();
     }
@@ -278,10 +276,9 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) {
         }
 
         const unfocusable = !disabledItemsFocusable && disabled;
-        if (!wasClick.current && !focused && event.currentTarget === event.target && !unfocusable) {
+        if (!focused && event.currentTarget === event.target && !unfocusable) {
           focus(event, nodeId);
         }
-        wasClick.current = false;
       };
 
       // Using focusin to avoid blurring the tree.

diff --git a/packages/material-ui-lab/src/TreeItem/TreeItem.test.js b/packages/material-ui-lab/src/TreeItem/TreeItem.test.js
@@ -1750,18 +1750,16 @@ describe('<TreeItem />', () => {
     describe('focus', () => {
       describe('`disabledItemsFocusable=true`', () => {
         it('should prevent focus by mouse', () => {
-          const { getByRole, getByText, getByTestId } = render(
-            <TreeView disabledItemsFocusable>
+          const focusSpy = spy();
+          const { getByText } = render(
+            <TreeView disabledItemsFocusable onNodeFocus={focusSpy}>
               <TreeItem nodeId="one" label="one" data-testid="one" />
               <TreeItem nodeId="two" label="two" disabled data-testid="two" />
             </TreeView>,
           );
 
           fireEvent.click(getByText('two'));
-          act(() => {
-            getByRole('tree').focus();
-          });
-          expect(getByTestId('two')).not.toHaveVirtualFocus();
+          expect(focusSpy.callCount).to.equal(0);
         });
 
         it('should not prevent programmatic focus', () => {
@@ -1828,18 +1826,16 @@ describe('<TreeItem />', () => {
 
       describe('`disabledItemsFocusable=false`', () => {
         it('should prevent focus by mouse', () => {
-          const { getByRole, getByText, getByTestId } = render(
-            <TreeView>
-              <TreeItem nodeId="one" label="one" disabled data-testid="one" />
-              <TreeItem nodeId="two" label="two" data-testid="two" />
+          const focusSpy = spy();
+          const { getByText } = render(
+            <TreeView onNodeFocus={focusSpy}>
+              <TreeItem nodeId="one" label="one" data-testid="one" />
+              <TreeItem nodeId="two" label="two" disabled data-testid="two" />
             </TreeView>,
           );
 
-          fireEvent.click(getByText('one'));
-          act(() => {
-            getByRole('tree').focus();
-          });
-          expect(getByTestId('one')).not.toHaveVirtualFocus();
+          fireEvent.click(getByText('two'));
+          expect(focusSpy.callCount).to.equal(0);
         });
 
         it('should prevent programmatic focus', () => {