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

fix: Allow mutable object refs to be used for FocusTrap child #72

Merged
merged 1 commit into from
Jun 20, 2020

Conversation

scottrippey
Copy link
Contributor

What

  • Fixes this.node.current is null. #50
  • Allows mutable object refs, created by React.createRef(), to be used on the first child of the <FocusTrap> component

Testing

Here is a simple sample component that demonstrates the problem and the fix.
The problem is more prevalent when using the newer React Hooks, since most refs will be mutable object refs instead of callbacks.

import React from 'react';
import FocusTrap from 'focus-trap-react';

class Example extends React.PureComponent {
  constructor(props) {
    super(props);
    this.ref = React.createRef();
  }
  
  componentDidMount() {
    if (!this.ref.current) {
      throw new Error('ref.current should be a reference to the button!');
    }
  }

  render() {
    return (
      <FocusTrap>
        <button ref={this.ref}>
          Button
        </button>
      </FocusTrap>
    );
  }
}

Copy link
Member

@stefcameron stefcameron left a comment

Choose a reason for hiding this comment

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

@scottrippey Thanks for this trivial fix! Looks good to me. I can confirm it fixes my issue with functional components as well.

@davidtheclark Any chance we could get this merged and published?

@stefcameron
Copy link
Member

@scottrippey Thanks for this PR! Merging...

@stefcameron stefcameron merged commit 312fcc7 into focus-trap:master Jun 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

this.node.current is null.
2 participants