Problem/Motivation
At certain screen widths (767px to 610px), the tab bar requires a second click to get the tray to open. After clicking a link in the tray, the tray is still visible and is hidden after the page loads.
Steps to reproduce
1. set screen to 700px wide and reload page
1. click Manage
1. notice that nothing happens
1. click Manage again
1. click Content
1. notice the tray is still visible briefly after the page loads
Proposed resolution
This module has some JS that alters toolbar classes on screens under 768px, but I don't think this works well with D10 anymore now that toolbar state is stored in the browser session.
Removing the JS makes the tray respond on the first click, and the tray is immediately hidden when you click a link. This matches the behaviour of screens below 610px.
Issue fork admin_toolbar-3452561
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
bwaindwain commentedMR created
Comment #4
bwaindwain commentedComment #5
pray_12 commentedThe above MR solves the issue of a second click being required for certain screen widths.
Comment #6
riddhi.addweb commentedI have applied the patch, it resolves the issue.
Please check the attachment for the reference.
Attachment : https://imgur.com/a/6u8N3sj
Comment #7
pray_12 commentedComment #9
dydave commentedThanks for finding this!
I've done a bit of testing and I "think" I was able to reproduce with the steps clearly detailed in the IS.
I need to do a bit of digging though to see where and why this code was initially added.
Needs a bit more time testing, but bringing this back to the top of the stack.
Thanks!
Comment #11
dydave commentedThanks a lot everyone for your great help on this issue! 🥳
Thank you very much for making the steps very clear in the IS:
Indeed, I was able to reproduce the issue and make a bit of digging around the impacted piece of code:
It seems the piece of code removed in MR!77 stems from an initial request in:
Issue #2991056: Always hide active menu item's navigation on mobile,
with commit:
https://git.drupalcode.org/project/admin_toolbar/-/commit/c1e705ff67b6d5...
which would seem to have "broken" module's previous JS behavior and led to:
Issue #3172430: Hide the navigation menu on mobile properly
with commit:
https://git.drupalcode.org/project/admin_toolbar/-/commit/6dd6516
We can't be sure whether this change is going to break the expected behavior on some sites, but since this piece of code is currently causing an issue it is probably safe to assume, these changes should be reverted for now.
Perhaps the way this feature was initially introduced in the module should have taken a different approach?!
Maybe it could have been added as a form setting or in a sub-module, for example.
Personally, I don't think it would be a good idea anyway to have a different behavior between larger and smaller devices.
Additionally, if this was the case, we would probably want to alter JS variables set in the Local Storage by the Core Toolbar module, instead of changing the DOM elements of the page.
Again, we wouldn't be closed or opposed to discussing the reverted feature again, if anybody would still be interested in working on this feature.
Since the jobs and tests were passing 🟢, I went ahead and merged the changes above at #10.
Marking issue as Fixed, for now.
Thanks again everyone for the great work and contributions! 🙏