reactjs / react-modal

Accessible modal dialog component for React
http://reactcommunity.org/react-modal
MIT License
7.35k stars 810 forks source link

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 2 years ago

shawnscoding commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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

shawnscoding commented 2 years ago

@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 commented 2 years ago

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

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

shawnscoding commented 2 years ago

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

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

diasbruno commented 2 years ago

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

brainsaysno commented 2 years ago

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 commented 1 year ago

@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 commented 1 year ago

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

blazkovicz commented 1 month ago

KeyDown handler which closes dialog should also do event.preventDefault(), then enter will not be processed on focused after dialog button.