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.

Command icon 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

bwaindwain created an issue. See original summary.

bwaindwain’s picture

Issue summary: View changes
Status: Active » Needs review

MR created

bwaindwain’s picture

Issue summary: View changes
pray_12’s picture

The above MR solves the issue of a second click being required for certain screen widths.

riddhi.addweb’s picture

I have applied the patch, it resolves the issue.
Please check the attachment for the reference.
Attachment : https://imgur.com/a/6u8N3sj

pray_12’s picture

Status: Needs review » Reviewed & tested by the community

dydave made their first commit to this issue’s fork.

dydave’s picture

Thanks 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!

  • dydave committed da09a6a7 on 3.x authored by bwaindwain
    Issue #3452561 by bwaindwain, dydave: Reverted changes from 'Hide the...
dydave’s picture

Thanks 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! 🙏

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.