From 6c456999995be47d0fd67983520b3c803d77ffe2 Mon Sep 17 00:00:00 2001
From: joshwooding <12938082+joshwooding@users.noreply.github.com>
Date: Tue, 28 Jul 2020 00:40:00 +0100
Subject: [PATCH] Seb feedback
---
.../pages/components/tree-view/tree-view.md | 4 +--
.../material-ui-lab/src/TreeItem/TreeItem.js | 7 ++---
.../src/TreeItem/TreeItem.test.js | 26 ++++++++-----------
3 files changed, 15 insertions(+), 22 deletions(-)
diff --git a/docs/src/pages/components/tree-view/tree-view.md b/docs/src/pages/components/tree-view/tree-view.md
index be6fe4ebce20ae..bd58f5a04dae34 100644
--- 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
index 05cece7b48d87b..2c47d627303d60 100644
--- 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
index 538554830d2c7d..9e84c325a0a4d0 100644
--- 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('', () => {
describe('focus', () => {
describe('`disabledItemsFocusable=true`', () => {
it('should prevent focus by mouse', () => {
- const { getByRole, getByText, getByTestId } = render(
-
+ const focusSpy = spy();
+ const { getByText } = render(
+
,
);
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('', () => {
describe('`disabledItemsFocusable=false`', () => {
it('should prevent focus by mouse', () => {
- const { getByRole, getByText, getByTestId } = render(
-
-
-
+ const focusSpy = spy();
+ const { getByText } = render(
+
+
+
,
);
- 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', () => {