Problem/Motivation

The navbar toggle uses '.navbar-collapse' as a selector value for its data-bs-target attribute, which is a class name already used by Bootstrap. So, any other Bootstrap-enabled menu on the page (with that class) will be controlled by the main menu toggle.

Steps to reproduce

• Create a page with items added to the Drupal Main menu, and add this menu to one of the navbar regions.
• Add a menu elsewhere according to example code from the Bootstrap documentation.
• Toggle the Drupal Main menu "hamburger" button.

Expected result: Drupal Main menu is expanded, other menu is unaffected.
Actual result: Both menus expand and collapse as the Drupal Main menu "hamburger" button is toggled.

Proposed resolution

Instead, the data-bs-target attribute should use a selector with an id attribute that is a name unique to the navbar it controls (which should have the same unique value for its id attribute).

Patch(s) change the '.navbar-collapse' selector to '#header-navbar' and adds this id to the div with the class '.navbar-collapse'

CommentFileSizeAuthor
navbar-toggles-all-menus-6-x-2.patch946 byteskroh
navbar-toggles-all-menus-5-x-2.patch862 byteskroh

Issue fork radix-3538947

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:

About issue forks

Comments

kroh created an issue. See original summary.

liam morland made their first commit to this issue’s fork.

liam morland opened merge request !157

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Title: navbar toggles all menus » Navbar button toggles all menus
Related issues: +#3572059: The `aria-controls` attribute references `navbar-collapse`, which does not exist

I have created a merge request with the provided patch.

Perhaps the ID should be radix-header-navbar. By using the theme name, it makes a conflict less likely.

doxigo’s picture

doxigo commented

Liam seems like we have some conflicts, can you maybe check? thank you

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

I have re-rolled the patch using radix-header-navbar.

doxigo closed merge request !157

doxigo’s picture

doxigo commented
Status: Needs review » Fixed

Thanks Liam, fixed it rather differently, given that the navbar might be reused and I don't want a hardcoded ID in there.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • doxigo committed 88770aa4 on 6.0.x 
    fix: #3538947 Navbar button toggles all menus

By: kroh
By: liam morland...
liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

Thanks!

Status: Fixed » Closed (fixed)

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