-
Notifications
You must be signed in to change notification settings - Fork 810
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
Focus should be restored to original position after closing modal #279
Comments
@benknight Thanks for filling this issue! In my use of the modal, I've generally made opening the modal set a variable at the modal parent's level that contained the button node (or id) that triggered opening the modal. Then when closing the modal I use the Would doing something like that meet your use case? If I added that to the demo page would that be sufficient? I guess it might be nice to have something a bit more explicit a11y-wise in the modal. What about making the modal take a |
I was thinking there might be a solution that required no callbacks or manual focus management on the author's part. Off the top of my head, it seems we could cache |
It would seem like that would be a good solution, but in practice it falls apart a bit. One particular thing that would fail there would be if the focus needs to go somewhere else because of an action in the modal. For instance, creating a new item inside a modal. After creation of the item, you should generally focus on the newly created item. But if you cancel the modal, you should go back to the trigger. I think it's really hard to generalize a single solution that works all the time :(. I'd probably lean towards having people provide where they need focus to go to be explicit about it. |
@claydiffrient I'm not seeing where it falls apart in your example, could you be more specific? If a new DOM node is created inside of Modal while it is mounted and focused, it would still be considered expected behavior for the original modal trigger (a DOM node that has been cached on the Modal instance as something like |
Sure, so let's imagine a list of students. Upon clicking on a "new student" button, a modal appears asking for information on the student to be added. Once the student is added, the modal closes. Focus would go to the newly created student on the list, but an alternative would be to send focus back to the "new student" button. I've seen this done both ways in different circumstances. But in either case, a blanket solution from the library is better left to the consumer of the library so they can do what works for them. Perhaps a better example/concern would be what happens if the trigger behind the modal gets removed? Say if you clicked a "delete student" button on the list item. A modal appears giving more information and asking for confirmation. If you delete the student, the modal closes, but then it's trigger is gone. Where does the focus go? Generally you'd send it to the item just previous to the item you deleted, but doing something like that is far out of the scope of what this library itself can do. Does that make things more clear? |
I do see how in your example that focusing the new student could be helpful. However what I am suggesting is changing the default behavior. The current default is to do nothing, thus leaving it 100% up to authors to decide how to do it. That means most authors will just do nothing. If the end goal however is to have this component be as accessible as possible by default, then restoring focus to whatever was active before the component was mounted is the correct thing to do in most cases I would argue*. At least I find that to be the case in my own experience.
|
@benknight Totally understand your request. We'll hash out an API for handling this. I'm putting it on our v2 roadmap. |
hey @diasbruno and @claydiffrient, I see this was removed from the 2.0 milestone, and we're now at 3.1. Any chance you plan on implementing this in a future version? |
@vanderhoop the focus manager should give back the focus to the previous element by default. We added the ability to prevent this behavior. If that is not happening then we need to check if it's a bug. |
Right now whenever my modal closes, it scrolls all the way left and does not return to the original div that called it. Ive tried every api callback and every react lifecycle method, and nothing works. However, if i use scrollIntoView in the terminal, it works just fine. Steps to Reproduce:
Here's my component that calls the modals
|
Can we prevent this from happening? It's working for me and is annoying to end users |
|
@evanjmg The idea of "return the focus" is for accessibility reasons. If there is a bug, please make a reproducible example so we can investigate. |
Summary:
For good keyboard a11y, when the user closes the modal, focus should be restored to whatever element was focused before they triggered the modal.
Steps to reproduce:
Expected behavior:
Focus should be restored to the button that opened the modal in step one.
The text was updated successfully, but these errors were encountered: