-
Notifications
You must be signed in to change notification settings - Fork 64
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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); |
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'); | ||
|
||
|
@@ -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) | ||
|
@@ -18,6 +26,19 @@ class FocusTrap extends React.Component { | |
} | ||
} | ||
|
||
getChildContext() { | ||
return { | ||
_FocusTrapReact: { | ||
pause: () => this.focusTrap.pause(), | ||
unpause: () => { | ||
if (this.props.paused === false) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's name this context prop something like There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 (pause) pause(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
|
@@ -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 => { | ||
|
@@ -96,6 +125,9 @@ class FocusTrap extends React.Component { | |
} | ||
} | ||
|
||
FocusTrap.contextTypes = FocusTrapReactContextType; | ||
FocusTrap.childContextTypes = FocusTrapReactContextType; | ||
|
||
FocusTrap.defaultProps = { | ||
active: true, | ||
tag: 'div', | ||
|
There was a problem hiding this comment.
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.