Large expanded menu closes when scrolling on mobile
bramchi opened this issue · 6 comments
When there is an elaborate menu with many nested menu items on multiple levels, a user is unable to scroll down on mobile when he/she has expanded some of these levels to the extend that they fall outside the viewport. In that situation the menu will close when scrolling.
I'd love to submit a pull-request that improves this, but I'm not sure where to look. Can you point me in the right direction? Does the menu lose focus when someone scrolls and therefor closes? I still see the hamburger icon having the :focus outline. I don't think responsive-menu.js has any influence on this.
Any pointers appreciated :-)
Hey @bramchi thanks for the issue!
I have also experienced the issue you describe. Luckily I was in a situation where I ended up needing a different menu solution, bit I'd like to get this fixed.
My hunch was that its something to do with the eventlisteners, but I haven't got a lot of time at the moment to test, so anything you find would be great.
I'd actually like move the menu out into its own package at some point too, so its not directly included in Luxe. I originally planned on having it here: https://github.com/brettsmason/a11y-responsive-menu
Thanks for your reply, @brettsmason. I'll dive in a bit deeper to see what I can find and hopefully fix.
Wonderful work btw, this theme. :-)
So here is what's happening:
The resize event fires when the mobile browser's address bar auto-hides when a user scrolls down. Since you use that resize event to reset the menu's state, it gets reset to the closed start position.
As a workaround I've disabled all those scroll event listeners and all is fine now.
I usually don't take that switching screen sizes scenario into account for my builds. Since this is your project, and you've put some amazing thought into the mobile menu JS, I'll leave it up to you how you want to adjust to this situation.
Cheers!
Thanks for digging in and taking a look - I appreciate it!
That sounds like a good solution. I'd like to revamp the menu script a bit as I've found it not as flexible as I'd like in real world projects. I still think it has potential though :)
Your focus/aria based accessibility approach is very inspiring and definitely has potential. I'm looking forward to see where you take it.
@bramchi I finally had some spare time to look at this.
I pushed a fix that keeps the responsive behaviour, but fixes this problem.
I still would like to turn this in to a separate package at some point, so if you have any feedback/requests for the menu at all it would be appreciated 😄