From 7b67699b3f74f0900676c3ca203d78493a44ad24 Mon Sep 17 00:00:00 2001 From: Ethan Standel Date: Sun, 17 Jul 2022 03:01:54 -0400 Subject: [PATCH 1/5] [base] Make invisible elements untabbable in TrapFocus when they are irrelevant (mui#33380) --- packages/mui-base/src/TrapFocus/TrapFocus.js | 11 +++++++--- .../mui-base/src/TrapFocus/TrapFocus.test.js | 22 +++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/packages/mui-base/src/TrapFocus/TrapFocus.js b/packages/mui-base/src/TrapFocus/TrapFocus.js index 30b20e59e13f53..09a8f72f77c80a 100644 --- a/packages/mui-base/src/TrapFocus/TrapFocus.js +++ b/packages/mui-base/src/TrapFocus/TrapFocus.js @@ -325,13 +325,18 @@ function TrapFocus(props) { return (
{React.cloneElement(children, { ref: handleRef, onFocus })} -
+
); } diff --git a/packages/mui-base/src/TrapFocus/TrapFocus.test.js b/packages/mui-base/src/TrapFocus/TrapFocus.test.js index d65f3951bab254..06edfda9a3703f 100644 --- a/packages/mui-base/src/TrapFocus/TrapFocus.test.js +++ b/packages/mui-base/src/TrapFocus/TrapFocus.test.js @@ -283,6 +283,28 @@ describe('', () => { expect(screen.getByTestId('root')).toHaveFocus(); }); + it('does not allow sentinelStart or sentinelEnd to be tabbable until open={true}', () => { + function Test(props) { + return ( + +
+ +
+ + ); + } + + const { setProps } = render(); + + setProps({ open: false }); + expect(screen.getByTestId('sentinelStart').getAttribute('tabindex')).to.eq('-1'); + expect(screen.getByTestId('sentinelEnd').getAttribute('tabindex')).to.eq('-1'); + + setProps({ open: true }); + expect(screen.getByTestId('sentinelStart').getAttribute('tabindex')).to.eq('0'); + expect(screen.getByTestId('sentinelEnd').getAttribute('tabindex')).to.eq('0'); + }); + describe('interval', () => { clock.withFakeTimers(); From b7c41a06a3ecb373ec08f4b0af1906ae53cfd31f Mon Sep 17 00:00:00 2001 From: Ethan Standel Date: Sun, 17 Jul 2022 03:02:04 -0400 Subject: [PATCH 2/5] [docs] adds use case for TrapFocus (mui#33380) --- .../trap-focus/ContainedToggleTrappedFocus.js | 33 +++++++++++++++++++ .../ContainedToggleTrappedFocus.tsx | 33 +++++++++++++++++++ .../ContainedToggleTrappedFocus.tsx.preview | 14 ++++++++ .../base/components/trap-focus/trap-focus.md | 7 ++++ 4 files changed, 87 insertions(+) create mode 100644 docs/data/base/components/trap-focus/ContainedToggleTrappedFocus.js create mode 100644 docs/data/base/components/trap-focus/ContainedToggleTrappedFocus.tsx create mode 100644 docs/data/base/components/trap-focus/ContainedToggleTrappedFocus.tsx.preview diff --git a/docs/data/base/components/trap-focus/ContainedToggleTrappedFocus.js b/docs/data/base/components/trap-focus/ContainedToggleTrappedFocus.js new file mode 100644 index 00000000000000..0fb77c38f0aeac --- /dev/null +++ b/docs/data/base/components/trap-focus/ContainedToggleTrappedFocus.js @@ -0,0 +1,33 @@ +import * as React from 'react'; +import Box from '@mui/material/Box'; +import Stack from '@mui/material/Stack'; +import TrapFocus from '@mui/base/TrapFocus'; + +export default function BasicTrapFocus() { + const [open, setOpen] = React.useState(false); + + return ( + + + + + + + {open && ( + + )} + + + + ); +} diff --git a/docs/data/base/components/trap-focus/ContainedToggleTrappedFocus.tsx b/docs/data/base/components/trap-focus/ContainedToggleTrappedFocus.tsx new file mode 100644 index 00000000000000..0fb77c38f0aeac --- /dev/null +++ b/docs/data/base/components/trap-focus/ContainedToggleTrappedFocus.tsx @@ -0,0 +1,33 @@ +import * as React from 'react'; +import Box from '@mui/material/Box'; +import Stack from '@mui/material/Stack'; +import TrapFocus from '@mui/base/TrapFocus'; + +export default function BasicTrapFocus() { + const [open, setOpen] = React.useState(false); + + return ( + + + + + + + {open && ( + + )} + + + + ); +} diff --git a/docs/data/base/components/trap-focus/ContainedToggleTrappedFocus.tsx.preview b/docs/data/base/components/trap-focus/ContainedToggleTrappedFocus.tsx.preview new file mode 100644 index 00000000000000..d748ddf123628c --- /dev/null +++ b/docs/data/base/components/trap-focus/ContainedToggleTrappedFocus.tsx.preview @@ -0,0 +1,14 @@ + + + + + + {open && ( + + )} + + \ No newline at end of file diff --git a/docs/data/base/components/trap-focus/trap-focus.md b/docs/data/base/components/trap-focus/trap-focus.md index 627624e3bde9ef..7e6a650d2d5d5b 100644 --- a/docs/data/base/components/trap-focus/trap-focus.md +++ b/docs/data/base/components/trap-focus/trap-focus.md @@ -84,3 +84,10 @@ When auto focus is disabled—as in the demo below—the component only traps th The following demo uses the [`Portal`](/base/react-portal/) component to render a subset of the `TrapFocus` children into a new "subtree" outside of the current DOM hierarchy, so they are no longer part of the focus loop: {{"demo": "PortalTrapFocus.js"}} + +## Using a toggle inside the trap + +The most common use case for the `TrapFocus` component is to maintain focus within a [`Modal`](/base/react-modal/) component which is entire separete from the element which would open the modal. However, there are use cases where you may want to have a toggle button for the `open` prop of the `TrapFocus` component which is stored inside that component. + +{{"demo": "ContainedToggleTrappedFocus.js"}} + From ecd60313b43daf0cbe3f0c819c77ea02c1973170 Mon Sep 17 00:00:00 2001 From: Ethan Standel Date: Wed, 27 Jul 2022 23:26:36 -0400 Subject: [PATCH 3/5] [base] adds test for non-tabbability of initial-state TrapFocus --- .../base/components/trap-focus/trap-focus.md | 1 - package.json | 1 + .../mui-base/src/TrapFocus/TrapFocus.test.js | 37 ++++++++++++------- yarn.lock | 5 +++ 4 files changed, 29 insertions(+), 15 deletions(-) diff --git a/docs/data/base/components/trap-focus/trap-focus.md b/docs/data/base/components/trap-focus/trap-focus.md index 7e6a650d2d5d5b..656ce4ec798bc8 100644 --- a/docs/data/base/components/trap-focus/trap-focus.md +++ b/docs/data/base/components/trap-focus/trap-focus.md @@ -90,4 +90,3 @@ The following demo uses the [`Portal`](/base/react-portal/) component to render The most common use case for the `TrapFocus` component is to maintain focus within a [`Modal`](/base/react-modal/) component which is entire separete from the element which would open the modal. However, there are use cases where you may want to have a toggle button for the `open` prop of the `TrapFocus` component which is stored inside that component. {{"demo": "ContainedToggleTrappedFocus.js"}} - diff --git a/package.json b/package.json index 88e6f6fe5c6120..8064e2512f00a6 100644 --- a/package.json +++ b/package.json @@ -88,6 +88,7 @@ "@rollup/plugin-replace": "^4.0.0", "@testing-library/dom": "^8.16.0", "@testing-library/react": "^13.3.0", + "@testing-library/user-event": "^14.3.0", "@types/chai": "^4.3.1", "@types/chai-dom": "^0.0.13", "@types/enzyme": "^3.10.12", diff --git a/packages/mui-base/src/TrapFocus/TrapFocus.test.js b/packages/mui-base/src/TrapFocus/TrapFocus.test.js index 06edfda9a3703f..26324e14f53c67 100644 --- a/packages/mui-base/src/TrapFocus/TrapFocus.test.js +++ b/packages/mui-base/src/TrapFocus/TrapFocus.test.js @@ -4,6 +4,7 @@ import { expect } from 'chai'; import { act, createRenderer, screen } from 'test/utils'; import TrapFocus from '@mui/base/TrapFocus'; import Portal from '@mui/base/Portal'; +import userEvent from '@testing-library/user-event'; describe('', () => { const { clock, render } = createRenderer(); @@ -283,26 +284,34 @@ describe('', () => { expect(screen.getByTestId('root')).toHaveFocus(); }); - it('does not allow sentinelStart or sentinelEnd to be tabbable until open={true}', () => { + it('does not create any tabbable elements when open={false}', () => { function Test(props) { return ( - -
- -
- +
+ + +
+ +
+ + +
); } - const { setProps } = render(); - - setProps({ open: false }); - expect(screen.getByTestId('sentinelStart').getAttribute('tabindex')).to.eq('-1'); - expect(screen.getByTestId('sentinelEnd').getAttribute('tabindex')).to.eq('-1'); + render(); - setProps({ open: true }); - expect(screen.getByTestId('sentinelStart').getAttribute('tabindex')).to.eq('0'); - expect(screen.getByTestId('sentinelEnd').getAttribute('tabindex')).to.eq('0'); + expect(screen.getByTestId('initial-focus')).toHaveFocus(); + act(() => { + userEvent.tab(); + }); + expect(screen.getByTestId('inside-focus')).toHaveFocus(); + act(() => { + userEvent.tab(); + }); + expect(screen.getByTestId('end-focus')).toHaveFocus(); }); describe('interval', () => { diff --git a/yarn.lock b/yarn.lock index 479e76be05157c..d6d03b0c64c3e6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3192,6 +3192,11 @@ "@testing-library/dom" "^8.5.0" "@types/react-dom" "^18.0.0" +"@testing-library/user-event@^14.3.0": + version "14.3.0" + resolved "https://registry.yarnpkg.com/@testing-library/user-event/-/user-event-14.3.0.tgz#0a6750b94b40e4739706d41e8efc2ccf64d2aad9" + integrity sha512-P02xtBBa8yMaLhK8CzJCIns8rqwnF6FxhR9zs810flHOBXUYCFjLd8Io1rQrAkQRWEmW2PGdZIEdMxf/KLsqFA== + "@theme-ui/color-modes@0.14.6": version "0.14.6" resolved "https://registry.yarnpkg.com/@theme-ui/color-modes/-/color-modes-0.14.6.tgz#68f70c6e976ab317e435059a0f9501ca607f1888" From 6976938d1dc9309f7564c19688562f75b8648380 Mon Sep 17 00:00:00 2001 From: Ethan Standel Date: Sun, 31 Jul 2022 22:12:53 -0400 Subject: [PATCH 4/5] [base] updates docs to match latest layout --- docs/data/base/components/trap-focus/trap-focus.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/data/base/components/trap-focus/trap-focus.md b/docs/data/base/components/trap-focus/trap-focus.md index 656ce4ec798bc8..258a0e0da76907 100644 --- a/docs/data/base/components/trap-focus/trap-focus.md +++ b/docs/data/base/components/trap-focus/trap-focus.md @@ -85,8 +85,9 @@ The following demo uses the [`Portal`](/base/react-portal/) component to render {{"demo": "PortalTrapFocus.js"}} -## Using a toggle inside the trap +### Using a toggle inside the trap -The most common use case for the `TrapFocus` component is to maintain focus within a [`Modal`](/base/react-modal/) component which is entire separete from the element which would open the modal. However, there are use cases where you may want to have a toggle button for the `open` prop of the `TrapFocus` component which is stored inside that component. +The most common use case for the `TrapFocus` component is to maintain focus within a [`Modal`](/base/react-modal/) component that is entirely separate from the element that opens the modal. +But you can also create a toggle button for the `open` prop of the `TrapFocus` component that is stored inside of the component itself, as shown in the following demo: {{"demo": "ContainedToggleTrappedFocus.js"}} From bf6dd7d8a75c36394bef7c3ccde77a95a9d74644 Mon Sep 17 00:00:00 2001 From: Ethan Standel Date: Tue, 9 Aug 2022 18:11:33 -0400 Subject: [PATCH 5/5] [docs] fix non-existant component reference to plain link --- docs/data/base/components/trap-focus/trap-focus.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/data/base/components/trap-focus/trap-focus.md b/docs/data/base/components/trap-focus/trap-focus.md index 258a0e0da76907..c35e010bfbcf92 100644 --- a/docs/data/base/components/trap-focus/trap-focus.md +++ b/docs/data/base/components/trap-focus/trap-focus.md @@ -87,7 +87,7 @@ The following demo uses the [`Portal`](/base/react-portal/) component to render ### Using a toggle inside the trap -The most common use case for the `TrapFocus` component is to maintain focus within a [`Modal`](/base/react-modal/) component that is entirely separate from the element that opens the modal. +The most common use case for the `TrapFocus` component is to maintain focus within a [modal](/base/react-modal/) component that is entirely separate from the element that opens the modal. But you can also create a toggle button for the `open` prop of the `TrapFocus` component that is stored inside of the component itself, as shown in the following demo: {{"demo": "ContainedToggleTrappedFocus.js"}}