Focus in mobile filters dialog
Describe the bug
In mobile view, the "Filters" button has aria-expanded="false", but this doesn't update to true when the menu is expanded. As this button opens a dialog rather than a simple menu, we don't need to use aria-expanded here. However, we need to make sure to keep the focus inside the modal while this is open.
To reproduce
Steps to reproduce the behavior:
- Log in as a business
- Go to http://localhost:3000/developers
- Inspect the page and switch to mobile view
- Turn on VoiceOver:
command + F5 - Tab to the "Filters" button and notice the screen reader announces "collapsed"
- Hit "Enter" and tab through the modal
- Keep tabbing through all the elements and notice that the focus moves outside the modal, back to the main page, while the modal is still open
- Turn VoiceOver off:
command + F5
Expected behavior
- Focus should be trapped inside modal while modal is open (should not move back to main page)
- When modal is closed, focus should go back to "Filters" button
- When "Filters" button focused, screen readers should not announce "collapsed"
Actual behavior
Once you tab through the elements in the modal, the focus goes back to the main page, outside modal When the modal is closed, the focus is lost and does not go back to "Filters"
Screenshots
https://user-images.githubusercontent.com/22390758/177101137-61fc8084-3730-4e6c-b443-ed9f49e005b3.mov
Additional context
Hi @joemasilotti Can I take up this issue and raise a PR to resolve it?
Yeah, sure thing. Thanks @atiqueakhtar!
Please see #658 for some ideas and notes. Most importantly, I think this can be accomplished with HTML markup only, no JavaScript.
Hi @joemasilotti can I pick this one up?
Yes, please. Thanks @DamonClark!
For the expected behavior: Focus should be trapped inside modal while modal is open (should not move back to main page). I am able to add a tab-index=-1 to the necessary element but then even if the modal is closed those elements aren't being tabbed. I don't know if there is a good way to do that without custom javascript.
Got it, thanks for digging in! What’s the minimal, most generic, JavaScript we could add to get that behavior.
@joemasilotti from what I'm researching I'm not really seeing any alternatives much better then what @atiqueakhtar implementation looked like. This is what I was considering adding https://hidde.blog/using-javascript-to-trap-focus-in-an-element/ but it looks very similar to what atiqueakhtar already committed.
Is there any way to set tab-index=-1 when on mobile vs. desktop so the options get skipped? I think that might save the custom JavaScript for listening for tabs and keys and such, right?
I think you could add something like this to the controller
def mobile?
request.user_agent =~ /android|blackberry|iphone|ipad|ipod|iemobile|mobile|webos/i
end
and then add a ternary operator that only passes tab-index=-1 on condition user_agent is a mobile device. That's all I'm seeing so far without javascript, was hoping for something a litter nicer.
I’m ok with some JavaScript. But I didn’t want to rebuild the entire tab selection engine.
Can we get viewport in the request? I’d rather chain it off of that since the media breakpoints are what determines if the mobile vs. desktop views are shown.
I have some changes I was going to try but I need permission to push. I fixed it so that "collapsed" wouldn't announce when filters button is focused. Then I started trying to create a viewPort condition so that it would set tabindex -1. But its not picking up the javascript for some reason. Once I can push maybe we can have a look.
Feel free to create a fork and push to that. No need for access to the existing PR or main repo.
Hey @joemasilotti I created a fork and pushed to it. My change isn't working but I think its the right idea. It uses minimal javascript to try and set the tabindex -1 to the elements outside of the modal when the view port is less then 768. If you or anyone else can have a look it would be much appreciated. Below is the commit changes. Thanks :)
https://github.com/joemasilotti/railsdevs.com/commit/2e22022544d97ff0289aaa4f317062a72aa28b6d
Awesome, thanks Damon! Can you submit a PR to RailssDevs based on that fork so I can take a look?
A few folks, myself included, have taken a pass at this with no luck.
I'm going to close this out for now so we can focus on other stuff.