Make WordPress Core

Opened 4 years ago

Closed 2 years ago

Last modified 2 years ago

#53331 closed defect (bug) (fixed)

Twenty Twenty-One: Bug in primary-navigation.js

Reported by: andreaboe's profile andreaboe Owned by: audrasjb's profile audrasjb
Milestone: 6.2 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch commit
Focuses: javascript Cc:

Description

The function that closes the menu and scrolls to anchor when an anchor link is clicked should target only clicks on anchor links in the menu.

Otherwise aria-expanded is toggled when an anchor link in the content area is clicked.

-Andrea

Attachments (1)

anchor-link-menu-button-toggled.png (19.5 KB) - added by sabernhardt 4 years ago.
menu toggle button, before and after clicking anchor link within content

Download all attachments as: .zip

Change History (25)

#1 @sabernhardt
4 years ago

  • Focuses javascript added
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

Thanks for the report!

I did not need to have the browser inspector open to see that the menu toggle button changes when clicking an anchor link within the content.

@sabernhardt
4 years ago

menu toggle button, before and after clicking anchor link within content

#2 @poena
3 years ago

Test instructions

In the WordPress admin backend, navigate to Appearance > Menus
Select a menu that shows in the "Primary Menu" Display Location. If there is no new menu, create a new one and assign it to the "Primary Menu" under Display Location
Create a new post or page. In the content, add text with an anchor link such as #test.
Go to the front of the website and open the post or page that you created.
Reduce the browser window width until the button that opens the responsive menu shows.
Click on the anchor link in the content.
See that the menu button changes and shows the text "Close" even though the menu is not open.

Test results

I am able to reproduce this using:
WordPress 6.0
Windows 10
Chrome version 102.0.5005.115


If I remember correctly, the JavaScript code that closes the menu when an anchor link is clicked was added to
support using on-page anchor links in the primary navigation.

#3 @poena
3 years ago

  • Owner set to poena
  • Status changed from new to assigned

This ticket was mentioned in PR #2833 on WordPress/wordpress-develop by carolinan.


3 years ago
#4

  • Keywords has-patch added; needs-patch removed

In Twenty Twenty-One, there is a bug where clicking on an anchor link in the content changes the state of the button that opens and closes the responsive menu.

This PR adds a conditional to check if the clicked anchor link is inside the primary navigation menu (ID site-navigation).
This is to prevent content anchor links from changing the state of the button.

Known limitations: The state of the button is not reset when the browser is resized.
If a menu anchor link is first clicked while on desktop width, and the browser width is reduced, the menu button will show the text "Close" even though the menu is not opened.

Trac ticket: https://core.trac.wordpress.org/ticket/53331

Testing instructions:

  1. In the WordPress admin backend, navigate to Appearance > Menus.
  2. Select a menu that shows in the "Primary Menu" Display Location. If there is no new menu, create a new one and assign it to the "Primary Menu" under Display Location.
  3. Under the "Add menu items" section, expand the sub-section "Custom Links".
  4. In the field URL, add something like #test.
  5. In the Field Link Text add something like "Sample Link".
  6. Click the Button Add To Menu to add the item to your menu.
  7. Click Save Menu which should be on the right near the bottom.
  8. Create a new post or page. In the content, add text with an anchor link such as #test2.
  9. Go to the front of the website and open the post or page that you created.
  10. Reduce the browser window width until the button that opens the responsive menu shows.
  11. Click on the anchor link in the content.
  12. Confirm that the menu button does not change visually.
  13. Open the primary menu in the header. Click on the Sample Link menu item you created in step 5.
  14. Confirm that the menu closes.

#5 @poena
3 years ago

  • Milestone changed from Future Release to 6.1

This ticket was mentioned in Slack in #core by chaion07. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by cu121. View the logs.


3 years ago

#8 @chaion07
3 years ago

  • Keywords needs-testing added

@andreaboe thank you for reporting this. We reviewed this during a recent bug-scrub session. Here's the summary of the feedback received:

  1. Added keyword needs-testing
  2. @cu121 was able to test and is in the process of adding a video with comment on this.
  3. There is still an issue left to be checked by the core committer, component maintainer or the owner of the ticket.

Cheers!

Props to @cu121 @robinwpdeveloper

#9 @cu121
3 years ago

Hello, I have made a video of the fix related to this ticket here is the video URL https://www.loom.com/share/e55d71f9e2574891834ea6a8cd0a70ad. I would like to emit two points
i) Based on the ticket description when the custom anchor link is clicked the menu button 'aria-expanded' property is not toggled when clicking the custom anchor
ii) By clicking the custom anchor URL, the button visually changes into white color.

If my testing process is incorrect, please feel free to leave feedback.

Thank You

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


3 years ago

#11 @sabernhardt
3 years ago

  • Milestone changed from 6.1 to 6.2

Let's revisit this in the next cycle.

This ticket was mentioned in Slack in #core-test by cu121. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-test by ironprogrammer. View the logs.


3 years ago

#14 @poena
3 years ago

I am not sure if I understand the testing results reported above correctly.
Point one is correct, the aria-expanded is working as expected.

@cu121 In the video, an external link in the menu is clicked. When clicked, the link background of that menu item is white.
Is that what you meant with point two? "By clicking the custom anchor URL, the button visually changes into white color."

-If I click the custom anchor that is on the page, not the menu, the menu button does not change to white.
-If I click on any link inside the menu, the menu button does not change to white.

With menu button, I mean the button that we click to open and close the responsive menu.
But I am not sure if that is what you meant. I tested in Chrome and Firefox and only with the default theme colors.

#15 @cu121
3 years ago

@poena yes I meant about clicking the anchor URLs of the menu in the twenty twenty-one theme. Really glad that point one is correct, the point two I mentioned was a bug, what I meant by this is that when I click the anchor URL on the menu, the button visually changes. I tested this issue which is actually not the main issue of this ticket, but this was mentioned here later with steps provided to reproduce the issue.

#16 @poena
3 years ago

Which button precisely? I can not see any button change visually in the recording.

#17 @cu121
3 years ago

@poena I think I missed a part of the test, I apologize for that. I was supposed to click the content of a post that contained a 'href' URL button. it seems that the menu button does not change visually, when the 'href' URL button is clicked on the page content, you are correct. Here is another video I constructed https://www.veed.io/view/7d1e77ab-d0b1-478c-95c7-b3b1e2d2e651/showcase.

#18 @poena
3 years ago

@cu121 No problem, thank you for clarifying.

#19 @mukesh27
3 years ago

@poena Thanks. PR looks good to me and approved.

#20 @poena
2 years ago

  • Keywords commit added; needs-testing removed

#21 @audrasjb
2 years ago

Thanks all! Self-assigning for commit.

#22 @audrasjb
2 years ago

  • Owner changed from poena to audrasjb
  • Status changed from assigned to accepted

#23 @audrasjb
2 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 55124:

Twenty Twenty-One: Refine primary-navigation.js behavior for anchor links.

This changeset fixes a bug where clicking on an anchor link in the content changes the state of the button that opens and closes the responsive menu. It adds a conditional to check if the clicked anchor link is inside the primary navigation menu (#site-navigation), in order to prevent content anchor links from changing the state of the button.

Known limitations: The state of the button is not reset when the browser is resized. If a menu anchor link is first clicked while on desktop width, and the browser width is reduced, the menu button will show the text "Close" even though the menu is not opened.

Props andreaboe, sabernhardt, poena, afercia, chaion07, cu121, mukesh27.
Fixes #53331.

Note: See TracTickets for help on using tickets.