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 EscKeyDownHandler bug in Container when closable is false #604

Merged
merged 1 commit into from
Sep 19, 2016

Conversation

kevinzwhuang
Copy link
Contributor

@kevinzwhuang kevinzwhuang commented Sep 12, 2016

Description:
There is a bug in lock when Container is set to container: "name" and the ESC button is pressed, this error occurs:

Uncaught TypeError: closeHandler is not a function - container.js:119
  handleClose @ container.js:119
  handleEsc @ container.js:128
  EscKeyDownHandler.handler @ container.js:54

The lock has the container UI option set to true, which should cause closable to change to false as well, as per the #Customization section in the docs - so this behavior should not be expected, since the lock is still expecting a function to handle close.

Changes Proposed:

  • Add condition to only mount Container component with EscKeyDownHandler to this.escKeydown when a closeHandler prop exists.
  • Add condition to only release this.escKeydown if this.escKeydown exists on Container unmount.

The changes above should prevent any further errors from the container attempting to close, when it should not be able to.

@kevinzwhuang
Copy link
Contributor Author

Thoughts on this bug fix @cristiandouce @hzalaz?

@cristiandouce
Copy link
Contributor

@kevinzwh thanks for the PR! I'll take a look at it tomorrow and merge.

Would you mind squashing your commits?

Thanks again!

…Handler prop exists

Add condition to only release escKeydown if it already exists on Container Unmount
@kevinzwhuang
Copy link
Contributor Author

Commits are squashed, thanks @cristiandouce

@cristiandouce cristiandouce merged commit 9e86896 into auth0:master Sep 19, 2016
@hzalaz hzalaz modified the milestone: v10.3.0 Sep 19, 2016
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.

3 participants