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

[#705] Fix setReturnFocus option as function not getting prev node #706

Merged
merged 2 commits into from
Jun 9, 2022
Merged
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
5 changes: 5 additions & 0 deletions .changeset/funny-turkeys-sleep.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'focus-trap-react': patch
---

Fix setReturnFocus option as function not being passed node focused prior to activation.
9 changes: 8 additions & 1 deletion demo/js/demo-setReturnFocus.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,14 @@ const DemoSetReturnFocusDialog = () => {

const focusTrapOptions = useMemo(
() => ({
setReturnFocus: '#AlternateReturnFocusElement',
setReturnFocus: (prevSelNode) => {
// contrived code to prove during tests that the setReturnFocus() option
// can be a function that is given a reference to the node that was focused
// prior to trap activation
return prevSelNode.parentNode.querySelector(
'#AlternateReturnFocusElement'
);
},
onDeactivate: () => setIsTrapActive(false),
}),
[]
Expand Down
71 changes: 50 additions & 21 deletions src/focus-trap-react.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ class FocusTrap extends React.Component {

// original options provided by the consumer
this.originalOptions = {
// because of the above `tailoredFocusTrapOptions`, we maintain our own flag for
// because of the above `internalOptions`, we maintain our own flag for
// this option, and default it to `true` because that's focus-trap's default
returnFocusOnDeactivate: true,

// because of the above `tailoredFocusTrapOptions`, we keep these separate since
// because of the above `internalOptions`, we keep these separate since
// they're part of the deactivation process which we configure (internally) to
// be shared between focus-trap and focus-trap-react
onDeactivate: null,
Expand All @@ -71,7 +71,7 @@ class FocusTrap extends React.Component {
optionName === 'clickOutsideDeactivates'
) {
this.originalOptions[optionName] = focusTrapOptions[optionName];
continue; // exclude from tailoredFocusTrapOptions
continue; // exclude from internalOptions
}

this.internalOptions[optionName] = focusTrapOptions[optionName];
Expand Down Expand Up @@ -106,36 +106,63 @@ class FocusTrap extends React.Component {
);
}

// TODO: Need more test coverage for this function
getNodeForOption(optionName) {
const optionValue = this.internalOptions[optionName];
if (!optionValue) {
return null;
/**
* Gets the node for the given option, which is expected to be an option that
* can be either a DOM node, a string that is a selector to get a node, `false`
* (if a node is explicitly NOT given), or a function that returns any of these
* values.
* @param {string} optionName
* @returns {undefined | false | HTMLElement | SVGElement} Returns
* `undefined` if the option is not specified; `false` if the option
* resolved to `false` (node explicitly not given); otherwise, the resolved
* DOM node.
* @throws {Error} If the option is set, not `false`, and is not, or does not
* resolve to a node.
*/
getNodeForOption = function (optionName, ...params) {
// use internal options first, falling back to original options
let optionValue =
this.internalOptions[optionName] ?? this.originalOptions[optionName];

if (typeof optionValue === 'function') {
optionValue = optionValue(...params);
}

let node = optionValue;
if (optionValue === true) {
optionValue = undefined; // use default value
}

if (typeof optionValue === 'string') {
node = this.getDocument()?.querySelector(optionValue);
if (!node) {
throw new Error(`\`${optionName}\` refers to no known node`);
if (!optionValue) {
if (optionValue === undefined || optionValue === false) {
return optionValue;
}
// else, empty string (invalid), null (invalid), 0 (invalid)

throw new Error(
`\`${optionName}\` was specified but was not a node, or did not return a node`
);
}

if (typeof optionValue === 'function') {
node = optionValue();
let node = optionValue; // could be HTMLElement, SVGElement, or non-empty string at this point

if (typeof optionValue === 'string') {
node = this.getDocument()?.querySelector(optionValue); // resolve to node, or null if fails
if (!node) {
throw new Error(`\`${optionName}\` did not return a node`);
throw new Error(
`\`${optionName}\` as selector refers to no known node`
);
}
}

return node;
}
};

getReturnFocusNode() {
const node = this.getNodeForOption('setReturnFocus');

return node ? node : this.previouslyFocusedElement;
const node = this.getNodeForOption(
'setReturnFocus',
this.previouslyFocusedElement
);
return node ? node : node === false ? false : this.previouslyFocusedElement;
}

/** Update the previously focused element with the currently focused element. */
Expand Down Expand Up @@ -395,12 +422,13 @@ FocusTrap.propTypes = {
initialFocus: PropTypes.oneOfType([
PropTypes.instanceOf(ElementType),
PropTypes.string,
PropTypes.func,
PropTypes.bool,
PropTypes.func,
]),
fallbackFocus: PropTypes.oneOfType([
PropTypes.instanceOf(ElementType),
PropTypes.string,
// NOTE: does not support `false` as value (or return value from function)
PropTypes.func,
]),
escapeDeactivates: PropTypes.oneOfType([PropTypes.bool, PropTypes.func]),
Expand All @@ -412,6 +440,7 @@ FocusTrap.propTypes = {
setReturnFocus: PropTypes.oneOfType([
PropTypes.instanceOf(ElementType),
PropTypes.string,
PropTypes.bool,
PropTypes.func,
]),
allowOutsideClick: PropTypes.oneOfType([PropTypes.bool, PropTypes.func]),
Expand Down
Loading