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

Jump to bottom

[#720] Add strict mode compatibility #721

Merged
merged 1 commit into from Jun 18, 2022
Merged

[#720] Add strict mode compatibility #721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/happy-knives-glow.md
View file
Open in desktop
@@ -0,0 +1,5 @@
---
'focus-trap-react': patch
---

Fix an issue when running in strict mode which has React immediately unmount/remount the trap, causing it to deactivate and then have to reactivate (per existing component state) on the remount. [#720](https://github.com/focus-trap/focus-trap-react/issues/720)
2 changes: 1 addition & 1 deletion demo/index.html
View file
Open in desktop
Expand Up @@ -70,7 +70,7 @@ <h2 id="special-element-heading">special element</h2>
arbitrary <code>data-</code>attribute.
</p>
<p>
Also, this FocusTrap activates and deactivates while staying mounted.
Also, this FocusTrap activates and deactivates while staying mounted (and does so in <a href="https://reactjs.org/docs/strict-mode.html" target="_blank">React 18 Strict Mode</a>, though the true effects of this are only applicable if you're running this demo <strong>locally in development mode</strong> since strict mode does not affect production code).
</p>
<p>
ALSO, when you click outside this FocusTrap, the trap deactivates and your click carries through.
Expand Down
82 changes: 42 additions & 40 deletions demo/js/demo-special-element.js
View file
Open in desktop
Expand Up @@ -9,7 +9,7 @@ class DemoSpecialElement extends React.Component {
super(props);

this.state = {
activeTrap: false,
activeTrap: true,
passThruMsg: '',
};

Expand Down Expand Up @@ -37,46 +37,48 @@ class DemoSpecialElement extends React.Component {
}

return (
<div>
<p>
<button
onClick={this.mountTrap}
aria-describedby="special-element-heading"
<React.StrictMode>
<div>
<p>
<button
onClick={this.mountTrap}
aria-describedby="special-element-heading"
>
activate trap
</button>
<button onClick={this.updatePassThruMsg}>pass thru click</button>
<span>{this.state.passThruMsg}</span>
</p>
<FocusTrap
active={this.state.activeTrap}
focusTrapOptions={{
onPostDeactivate: this.unmountTrap,
clickOutsideDeactivates: true,
returnFocusOnDeactivate: true,
}}
>
activate trap
</button>
<button onClick={this.updatePassThruMsg}>pass thru click</button>
<span>{this.state.passThruMsg}</span>
</p>
<FocusTrap
active={this.state.activeTrap}
focusTrapOptions={{
onPostDeactivate: this.unmountTrap,
clickOutsideDeactivates: true,
returnFocusOnDeactivate: true,
}}
>
<section
id="focus-trap-three"
style={this.state.activeTrap ? null : { background: '#eee' }}
data-whatever="nothing"
className={trapClass}
>
<p>
Here is a focus trap <a href="#">with</a> <a href="#">some</a>{' '}
<a href="#">focusable</a> parts.
</p>
<p>
<button
onClick={this.unmountTrap}
aria-describedby="special-element-heading"
>
deactivate trap
</button>
</p>
</section>
</FocusTrap>
</div>
<section
id="focus-trap-three"
style={this.state.activeTrap ? null : { background: '#eee' }}
data-whatever="nothing"
className={trapClass}
>
<p>
Here is a focus trap <a href="#">with</a> <a href="#">some</a>{' '}
<a href="#">focusable</a> parts.
</p>
<p>
<button
onClick={this.unmountTrap}
aria-describedby="special-element-heading"
>
deactivate trap
</button>
</p>
</section>
</FocusTrap>
</div>
</React.StrictMode>
);
}
}
Expand Down
6 changes: 3 additions & 3 deletions package.json
View file
Open in desktop
Expand Up @@ -14,8 +14,8 @@
"index.d.ts"
],
"scripts": {
"demo-bundle": "browserify demo/js -t babelify --extension=.jsx -o demo/demo-bundle.js",
"start": "yarn build && budo demo/js/index.js:demo-bundle.js --dir demo --live -- -t babelify --extension=.jsx",
"demo-bundle": "NODE_ENV=production browserify demo/js -t babelify --extension=.jsx -o demo/demo-bundle.js",
"start": "yarn build && NODE_ENV=development budo demo/js/index.js:demo-bundle.js --dir demo --live -- -t babelify --extension=.jsx",
"lint": "eslint \"*.js\" \"src/**/*.js\" \"test/**/*.js\" \"demo/**/*.js\" \"cypress/**/*.js\"",
"format": "prettier --write \"{*,src/**/*,test/**/*,demo/js/**/*,.github/workflows/*,cypress/**/*}.+(js|yml)\"",
"format:check": "prettier --check \"{*,src/**/*,test/**/*,demo/js/**/*,.github/workflows/*,cypress/**/*}.+(js|yml)\"",
Expand Down Expand Up @@ -89,7 +89,7 @@
"prettier": "^2.7.0",
"prop-types": "^15.8.1",
"react": "^18.2.0",
"react-dom": "^18.1.0",
"react-dom": "^18.2.0",
"regenerator-runtime": "^0.13.9",
"start-server-and-test": "^1.14.0",
"typescript": "^4.7.3"
Expand Down
22 changes: 21 additions & 1 deletion src/focus-trap-react.js
View file
Open in desktop
Expand Up @@ -270,7 +270,27 @@ class FocusTrap extends React.Component {
}

setupFocusTrap() {
if (!this.focusTrap) {
if (this.focusTrap) {
// trap already exists: it's possible we're in StrictMode and we're being remounted,
// in which case, we will have deactivated the trap when we got unmounted (remember,
// StrictMode, in development, purposely unmounts and remounts components after
// mounting them the first time to make sure they have reusable state,
// @see https://reactjs.org/docs/strict-mode.html#ensuring-reusable-state) so now
// we need to restore the state of the trap according to our component state
// NOTE: Strict mode __violates__ assumptions about the `componentWillUnmount()` API
// which clearly states -- even for React 18 -- that, "Once a component instance is
// unmounted, __it will never be mounted again.__" (emphasis ours). So when we get
// unmounted, we assume we're gone forever and we deactivate the trap. But then
// we get remounted and we're supposed to restore state. But if you had paused,
// we've now deactivated (we don't know we're amount to get remounted again)
// which means we need to reactivate and then pause. Otherwise, do nothing.
if (this.props.active && !this.focusTrap.active) {
this.focusTrap.activate();
if (this.props.paused) {
this.focusTrap.pause();
}
}
} else {
const nodesExist = this.focusTrapElements.some(Boolean);
if (nodesExist) {
// eslint-disable-next-line react/prop-types -- _createFocusTrap is an internal prop
Expand Down
18 changes: 9 additions & 9 deletions yarn.lock
View file
Open in desktop
Expand Up @@ -7504,13 +7504,13 @@ range-parser@~1.2.1:
resolved "https://registry.yarnpkg.com/range-parser/-/range-parser-1.2.1.tgz#3cf37023d199e1c24d1a55b84800c2f3e6468031"
integrity sha512-Hrgsx+orqoygnmhFbKaHE6c296J+HTAQXoxEF6gNupROmmGJRoyzfG3ccAveqCBrwr/2yxQ5BVd/GTl5agOwSg==

react-dom@^18.1.0:
version "18.1.0"
resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-18.1.0.tgz#7f6dd84b706408adde05e1df575b3a024d7e8a2f"
integrity sha512-fU1Txz7Budmvamp7bshe4Zi32d0ll7ect+ccxNu9FlObT605GOEB8BfO4tmRJ39R5Zj831VCpvQ05QPBW5yb+w==
react-dom@^18.2.0:
version "18.2.0"
resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-18.2.0.tgz#22aaf38708db2674ed9ada224ca4aa708d821e3d"
integrity sha512-6IMTriUmvsjHUjNtEDudZfuDQUoWXVxKHhlEGSk81n4YFS+r/Kl99wXiwlVXtPBtJenozv2P+hxDsw9eA7Xo6g==
dependencies:
loose-envify "^1.1.0"
scheduler "^0.22.0"
scheduler "^0.23.0"

react-is@^16.13.1:
version "16.13.1"
Expand Down Expand Up @@ -7920,10 +7920,10 @@ saxes@^5.0.1:
dependencies:
xmlchars "^2.2.0"

scheduler@^0.22.0:
version "0.22.0"
resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.22.0.tgz#83a5d63594edf074add9a7198b1bae76c3db01b8"
integrity sha512-6QAm1BgQI88NPYymgGQLCZgvep4FyePDWFpXVK+zNSUgHwlqpJy8VEh8Et0KxTACS4VWwMousBElAZOH9nkkoQ==
scheduler@^0.23.0:
version "0.23.0"
resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.23.0.tgz#ba8041afc3d30eb206a487b6b384002e4e61fdfe"
integrity sha512-CtuThmgHNg7zIZWAXi3AsyIzA3n4xx7aNyjwC2VJldO2LMVDhFK+63xGqq6CsJH4rTAt6/M+N4GhZiDYPx9eUw==
dependencies:
loose-envify "^1.1.0"

Expand Down