Make WordPress Core

Opened 3 years ago

Last modified 4 weeks ago

#52694 new defect (bug)

Twenty Twenty-One: Primary menu toggle filter adds toggles to third party menu locations

Reported by: domainsupport's profile domainsupport Owned by:
Milestone: 6.6 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch needs-testing
Focuses: Cc:

Description

The code in /inc/menu-functions.php ...

<?php
function twenty_twenty_one_add_sub_menu_toggle( $output, $item, $depth, $args ) {
        if ( 0 === $depth && in_array( 'menu-item-has-children', $item->classes, true ) ) {

                // Add toggle button.
                $output .= '<button class="sub-menu-toggle" aria-expanded="false" onClick="twentytwentyoneExpandSubMenu(this)">';
                $output .= '<span class="icon-plus">' . twenty_twenty_one_get_icon_svg( 'ui', 'plus', 18 ) . '</span>';
                $output .= '<span class="icon-minus">' . twenty_twenty_one_get_icon_svg( 'ui', 'minus', 18 ) . '</span>';
                $output .= '<span class="screen-reader-text">' . esc_html__( 'Open menu', 'twentytwentyone' ) . '</span>';
                $output .= '</button>';
        }
        return $output;
}
add_filter( 'walker_nav_menu_start_el', 'twenty_twenty_one_add_sub_menu_toggle', 10, 4 );

... should be updated so it doesn't affect third party menu locations.

Perhaps by using ...

<?php
if (isset($args->theme_location) && $args->theme_location === 'primary') {
...
}

Thank you.

Oliver

Attachments (1)

52694.diff (831 bytes) - added by domainsupport 7 weeks ago.

Download all attachments as: .zip

Change History (11)

#1 @mukesh27
3 years ago

I like the approach of @domainsupport. We can marge theme location condition in if statement like below.

if ( isset( $args->theme_location ) && 'primary' === $args->theme_location && 0 === $depth && in_array( 'menu-item-has-children', $item->classes, true ) ) {

This ticket was mentioned in PR #1066 on WordPress/wordpress-develop by mukeshpanchal27.


3 years ago
#2

  • Keywords has-patch added

#3 @mukesh27
3 years ago

Ping @poena for thoughts and progress for this ticket.

#4 @poena
2 years ago

Hi @mukesh27, do you want to create a new pull request?

#5 @poena
2 years ago

  • Keywords has-patch removed

#6 @karmatosed
7 weeks ago

  • Keywords needs-patch added

@domainsupport
7 weeks ago

#7 @domainsupport
7 weeks ago

  • Keywords has-patch added; needs-patch removed

#8 @poena
6 weeks ago

  • Keywords needs-testing-info needs-testing added
  • Milestone changed from Awaiting Review to 6.6

#9 @domainsupport
6 weeks ago

  • Keywords needs-testing-info removed

Testing Instructions

These steps define how to reproduce the issue, and indicate the expected behavior.

The easiest way is to use a legacy Navigation Menu widget as follows ...

Steps to Reproduce

  1. Install WordPress.
  2. Activate Twenty Twenty-One theme
  3. Install and activate "Classic Widgets" plugin
  4. Create a new Menu with a parent menu item and child menu item
  5. Add a legacy "Navigation Menu" widget to the Footer widget area and select your menu
  6. View the front page and examine the footer widget HTML
  7. 🐞 You will see there is a button element that shouldn't be there ...
<button class="sub-menu-toggle" aria-expanded="false" onclick="twentytwentyoneExpandSubMenu(this)"><span class="icon-plus"><svg class="svg-icon" width="18" height="18" aria-hidden="true" role="img" focusable="false" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"><path fill-rule="evenodd" clip-rule="evenodd" d="M18 11.2h-5.2V6h-1.6v5.2H6v1.6h5.2V18h1.6v-5.2H18z" fill="currentColor"></path></svg></span><span class="icon-minus"><svg class="svg-icon" width="18" height="18" aria-hidden="true" role="img" focusable="false" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"><path fill-rule="evenodd" clip-rule="evenodd" d="M6 11h12v2H6z" fill="currentColor"></path></svg></span><span class="screen-reader-text">Open menu</span></button>

Expected Results

When testing a patch to validate it works as expected:

  • ✅ No button element should be appended to the menu outside of the primary menu location

When reproducing a bug:

  • button element appears

Test Report Icons:
🐞 <= Indicates where issue ("bug") occurs.
✅ <= Behavior is expected.
❌ <= Behavior is NOT expected.

This ticket was mentioned in Slack in #core-themes by karmatosed. View the logs.


4 weeks ago

Note: See TracTickets for help on using tickets.