WordPress.org

Make WordPress Core

Opened 11 months ago

Closed 11 months ago

Last modified 11 months ago

#39026 closed defect (bug) (fixed)

Twenty Seventeen: active submenu toggle button initial state should have aria expanded true

Reported by: afercia Owned by: helen
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.7
Component: Bundled Theme Keywords: has-screenshots has-patch
Focuses: Cc:

Description

In Twenty Seventeen, when in the responsive view and when the current menu item is a subitem, the menu shows the active tree expanded. By the way, the submenu toggle button has a wrong initial state. When setting the initial state, 'aria-expanded="true"` should be set together with the CSS class 'toggled-on'.

See in the screenshot below: the current page is "Page Markup And Formatting", the tree it belongs to is expanded, the related toggle button has the proper icon but the wrong aria-expanded value:

https://cldup.com/gdiPiGbWyE.png

Attachments (3)

39026.patch (841 bytes) - added by laurelfulford 11 months ago.
39026.1.patch (1.1 KB) - added by laurelfulford 11 months ago.
39026.diff (1.1 KB) - added by afercia 11 months ago.

Download all attachments as: .zip

Change History (11)

#1 @afercia
11 months ago

P.S. the screen reader text should have the correct text too, in this case it should be Collapse child menu.

#2 @davidakennedy
11 months ago

  • Milestone changed from Awaiting Review to 4.7

#3 @laurelfulford
11 months ago

  • Keywords has-patch added; needs-patch removed

39026.patch only addressed the aria-expanded issue - 39026.1.patch should set the aria-expanded attribute to the correct state, as well as update the button screen reader text in cases where the current menu item is a subitem.

#4 @davidakennedy
11 months ago

  • Keywords commit added

Nice catch @afercia – thank you! I tested 39026.1.patch from @laurelfulford and it works well. This should be good to go.

@afercia
11 months ago

#5 @afercia
11 months ago

Thanks @laurelfulford and @davidakennedy I'd just suggest to don't get again .current-menu-ancestor > button and make it in one go. The refreshed patch still needs an additional find() but it's a bit more concise.

#6 @davidakennedy
11 months ago

  • Keywords commit removed

Noted in #39029, I'd rather go with the patch there from @afercia instead of this one. That patch would also fix the issue here in a similar way.

#7 @helen
11 months ago

  • Owner set to helen
  • Resolution set to fixed
  • Status changed from new to closed

In 39451:

Twenty Seventeen: Improve ARIA for the nav menu.

The onResizeARIA() function was not very useful and buggy.

props afercia.
fixes #39029, #39026.

#8 @helen
11 months ago

In 39452:

Twenty Seventeen: Improve ARIA for the nav menu.

The onResizeARIA() function was not very useful and buggy.

props afercia.
fixes #39029, #39026 for the 4.7 branch.

Note: See TracTickets for help on using tickets.