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

Pressing enter key to close button closes the modal but the event is attached to opener element and reopens the modal #954

Open
shawnscoding opened this issue Jun 18, 2022 · 9 comments

Comments

@shawnscoding
Copy link

Summary:

Steps to reproduce:

  1. Visit my demo on Codesandbox https://codesandbox.io/s/react-modal-vivyxc?file=/src/index.js
  2. Click 'Open Modal' button and press Tab. This will focus the close button
  3. Press Enter this closes the modal but reopens it. I know this is because of the onKeyUp event listener to anchor tag but this function is needed for other purposes.

Expected behavior:

Closes modal and focus move to the opener element( in this case, anchor tag)

Link to example of issue:

Codesandbox https://codesandbox.io/s/react-modal-vivyxc?file=/src/index.js

Additional notes:

Is there a way to solve this issue without removing the onKeyUp event handler?

Thanks.

@diasbruno
Copy link
Collaborator

@shawnscoding This is a very weird behavior...

When you hit enter to close the modal, react-modal uses the onKeyDown event to start the closing process, and, when
it will give back the focus to the element that opens the modal.

Since you add a onKeyUp handler, when the enter is released, this handler is triggered.

One fix is to change the your handler to use onKeyDown, instead of onKeyUp.

@PaoloJN
Copy link

PaoloJN commented Jun 19, 2022

Hi @shawnscoding, Are you still experiencing this issue or is it resolved ?

@shawnscoding
Copy link
Author

@diasbruno
Thank you for your suggestion. However, unfortunately, there are hundreds of anchor element modal openers in our react project.
They all have the onKeyUp event handler. I might look lazy but I'd like to have a chance to resolve this issue without modifying the anchor elements. Because that would be much simpler in my case.

Hi @PaoloJN
I am still experiencing this issue.

@diasbruno
Copy link
Collaborator

Sometimes we pay the price for ctrl+c ctrl-v. 😂

If you know grep and sed or awk or comby this is a very simple problem to fix. Give this tools a try...

@shawnscoding
Copy link
Author

shawnscoding commented Jun 19, 2022

@diasbruno I understand that. Thank you for your help :)

If this happens to be fixed later on, please let me know.

@diasbruno
Copy link
Collaborator

Try comby. It's simple and amazing refactoring tool.

@brainsaysno
Copy link

Your issue seems to be specific to your project rather than being a package issue. +1 on refactoring to make your events onKeyDown. That solves it.

@prajwal-np
Copy link

prajwal-np commented Mar 2, 2023

@shawnscoding I have made some changes to codesandbox which has resolved the issue. Please find the solution here https://codesandbox.io/s/react-modal-forked-t35fts?file=/src/index.js

@aasimtaif
Copy link

change setIsOpen(true) to setIsOpen(!modalIsOpen) in openModal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants