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.

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

Add code & demo to pause/unpause parents of nested traps #30

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions demo/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ <h2>demo autofocus</h2>
</p>
<div id="demo-autofocus"></div>

<h2>nested traps</h2>
<p>
The trap contains a nested child trap. The parent trap automatically pauses when the child is active.
</p>
<div id="demo-nested"></div>

<p>
<span style="font-size:2em;vertical-align:middle;">☜</span>
<a href="https://github.com/davidtheclark/focus-trap-react" style="vertical-align:middle;">Return to the repository</a>
Expand Down
86 changes: 86 additions & 0 deletions demo/js/demo-nested.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
const React = require('react');
const ReactDOM = require('react-dom');
const FocusTrap = require('../../dist/focus-trap-react');

const container = document.getElementById('demo-nested');

class DemoNestedTrap extends React.Component {
constructor(props) {
super(props);

this.state = {
activeParentTrap: false,
activeChildTrap: false,
};

this.mountParentTrap = this.mountParentTrap.bind(this);
this.unmountParentTrap = this.unmountParentTrap.bind(this);
this.mountChildTrap = this.mountChildTrap.bind(this);
this.unmountChildTrap = this.unmountChildTrap.bind(this);
}

mountParentTrap() {
this.setState({ activeParentTrap: true });
}

unmountParentTrap() {
this.setState({ activeParentTrap: false });
}

mountChildTrap() {
this.setState({ activeChildTrap: true });
}

unmountChildTrap() {
this.setState({ activeChildTrap: false });
}

render() {
const childTrap = this.state.activeChildTrap
? <FocusTrap>
<div className="trap">
<button onClick={this.unmountChildTrap}>
deactivate child trap
</button>
<div>
<a href="#">Another focusable thing</a>
</div>
</div>
</FocusTrap>
: false;

const parentTrap = this.state.activeParentTrap
? <FocusTrap
focusTrapOptions={{
onDeactivate: this.unmountParentTrap
}}
>
<div className="trap">
<button onClick={this.unmountParentTrap}>
deactivate parent trap
</button>
<div>
<a href="#">Another focusable thing</a>
</div>
<button onClick={this.mountChildTrap}>
activate child trap
</button>
{childTrap}
</div>
</FocusTrap>
: false;

return (
<div>
<p>
<button key="button" onClick={this.mountParentTrap}>
activate parent trap
</button>
</p>
{parentTrap}
</div>
);
}
}

ReactDOM.render(<DemoNestedTrap />, container);
1 change: 1 addition & 0 deletions demo/js/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ require('./demo-defaults');
require('./demo-ffne');
require('./demo-special-element');
require('./demo-autofocus');
require('./demo-nested');
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,15 @@
"eslint-plugin-react": "^6.10.3",
"jest": "^23.4.0",
"prettier": "^1.2.2",
"prop-types": "^15.0.0",
"react": "^16.0.0",
"react-dom": "^16.0.0"
},
"dependencies": {
"focus-trap": "^3.0.0"
},
"peerDependencies": {
"prop-types": "^15.0.0",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It this absolutely necessary, or could we avoid the breaking change by providing a dummy function for the context types?

Maybe we should end up adding a PropTypes peer dependency, but I'd love to treat that as a separate issue if possible.

"react": "0.14.x || ^15.0.0 || ^16.0.0",
"react-dom": "0.14.x || ^15.0.0 || ^16.0.0"
},
Expand Down
32 changes: 32 additions & 0 deletions src/focus-trap-react.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const PropTypes = require('prop-types');
const React = require('react');
const createFocusTrap = require('focus-trap');

Expand All @@ -9,6 +10,13 @@ const checkedProps = [
'_createFocusTrap'
];

const FocusTrapReactContextType = {
_FocusTrapReact: PropTypes.shape({
pause: PropTypes.func.isRequired,
unpause: PropTypes.func.isRequired,
})
};

class FocusTrap extends React.Component {
constructor(props) {
super(props)
Expand All @@ -18,6 +26,19 @@ class FocusTrap extends React.Component {
}
}

getChildContext() {
return {
_FocusTrapReact: {
pause: () => this.focusTrap.pause(),
unpause: () => {
if (this.props.paused === false) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could leave out this condition, since focus-trap already handles it: https://github.com/davidtheclark/focus-trap/blob/master/index.js#L93

this.focusTrap.unpause()
}
}
}
};
}

componentDidMount() {
// We need to hijack the returnFocusOnDeactivate option,
// because React can move focus into the element before we arrived at
Expand Down Expand Up @@ -45,6 +66,10 @@ class FocusTrap extends React.Component {
if (this.props.paused) {
this.focusTrap.pause();
}

// if there is a _FocusTrapReact context from a parent focus trap, pause it
const {_FocusTrapReact: {pause} = {}} = this.context;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's name this context prop something like FocusTrapParent, to clearly indicate that we're talking about a parent.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer some more straightforward non-destructured code here — something like if (this.context.FocusTrapParent && this.context.FocusTrapParent.pause) { .. } — especially because this instance has its own pause function.

if (pause) pause();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I recall right, focus-trap itself should already pause any active focus traps when a new once is activated: https://github.com/davidtheclark/focus-trap/blob/6ecf7575401b2050cf2f3935544b82d75155c560/index.js#L101-L105. Does that work in this context? If so, maybe we could remove this part of the change?

Then the only trick would be unpausing the previously active trap — similar to this focus-trap demo: https://github.com/davidtheclark/focus-trap/blob/6ecf7575401b2050cf2f3935544b82d75155c560/demo/js/nested.js

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That actually makes me wonder if this is how all focus-traps should behave, whether in React or not. If the activation of trap B ends up pausing trap A, should the deactivation of trap B always end up unpausing trap B?

}

componentDidUpdate(prevProps) {
Expand All @@ -70,6 +95,10 @@ class FocusTrap extends React.Component {
) {
this.previouslyFocusedElement.focus();
}

// if there is a _FocusTrapReact context from a parent focus trap, activate it
const {_FocusTrapReact: {unpause} = {}} = this.context;
if (unpause) unpause();
}

setNode = el => {
Expand All @@ -96,6 +125,9 @@ class FocusTrap extends React.Component {
}
}

FocusTrap.contextTypes = FocusTrapReactContextType;
FocusTrap.childContextTypes = FocusTrapReactContextType;

FocusTrap.defaultProps = {
active: true,
tag: 'div',
Expand Down