Twenty Twenty: Search modal aria-expanded

[utz119]

Theme: Twenty Twenty

When opening the search modal using the Tab key, then closing it using the Esc key causes aria-expanded to remain “true” even after the modal is closed.

To reproduce the issue:

1. Navigate to the search button using the Tab key
2. Use the Enter key to open the search form
3. Use the Esc key to close

[mukesh27]

Hi and welcome to WordPress Trac! Thanks for the ticket.

I reproduce the issue.

Additionally, if we click outside or the form it show same aria-expanded.

[Hareesh Pillai]

Thanks for the ticket and patch @utz119.

It is noteworthy that the aria-expanded attribute is set correctly when the search modal is clicked in the responsive screen sizes. It is handled through the twentytwentyToggleAttribute() function. We should perhaps refresh the patch to make use of the currently written code.

[sabernhardt]

With either the desktop or mobile search toggle button, the patch correctly toggles the aria-expanded attribute to false when pressing the Escape key. The desktop search toggle button also switches to false when clicking outside the modal to close it.

However, the mobile search button seems to toggle twice the first time I click outside the modal, which results in a true value when the modal is closed. Then clicking the mobile search button and closing by clicking outside again, the aria-expanded value continues to toggle to the incorrect value.

The code below does not fix the double toggling, but it might be worth considering for these two situations because the value should always become false.

			document.querySelectorAll( '.search-toggle' ).forEach( function( element ) {
				element.setAttribute( 'aria-expanded', 'false' );
			} );

(Accurate toggling is probably trickier due to the two search buttons.)

[alexstine]

I uploaded 53951-2.diff and I believe it now works correctly with Escape key. I am not sure about the click outside because that is hard to do for screen reader, but I was able to get Escape working I think.

Does this patch work any better?

Thanks.

[sabernhardt]

53951-2.diff does not change the aria-expanded to false for me when pressing the Escape key (on desktop).

With only a few hours before Beta, let's revisit this later, for next release.

[alexstine]

@sabernhardt Does pressing Escape only work on mobile or does it not work at all? My tests showed it working fine but interested in the specifics. I'll take ownership of this ticket and make sure we get something that works for 6.0.

Thanks.

[alexstine]

Just uploaded 53951-3.diff. Instead of playing around with aria-expanded further, I made the search form a proper dialog. I believe this is a much more accessible approach. Would be interested in getting others feedback.

Thanks.

[sabernhardt]

53951-3.diff seems to work.

With Firefox and NVDA, I press Escape twice to close the dialog when focus is in the search input, which was also necessary before the patch. However, then I need to press Enter twice on the Search button (either mobile or desktop) to re-open the dialog.

[alexstine]

@sabernhardt This should now be fixed in latest patch. After pressing Escape twice to exit the modal, Enter once should open it back up. Tested for both desktop/mobile.

[audrasjb]

53951-4.diff works when pressing esc twice, but it doesn't remove the aria-expanded="true" attribute when I only press esc once. Is it expected?

[afercia]

Looking a bit into this issue, seems to me the root problem is that aria-expanded needs to be handled on multiple elements at once. Ideally, the state of the toggle search buttons for mobile and desktop should be synced.

53951.2.diff​ changes the twentytwentyToggleAttribute function so that:

• It selects all the toggles with the same data-toggle-target attribute of the passed element.
• Loops through the collected toggles to update their aria-expanded attribute.

I kept the role="dialog" added in previous patches and also added an aria-label to the dialog.

Some testing would be greatly appreciated. Things to test:

• Both the desktop and mobile Search toggle buttons get their aria-expanded attribute updated, in all cases: click, Escape key press, click outside. On both the desktop and responsive view.
• Check the mobile menu toggle still works as expected.
• Check the mobile sub-menus toggles still work as expected.

[alexstine]

In 53951.3.diff, I made one improvement to ensure the close search button did not receive aria-expanded attribute. This is one extra conditional check added to @afercia patch.

[afercia]

Thanks @alexstine, good catch.

I just noticed there's one more issue related to another button. When the mobile navigation menu is open, there's a 'Close menu' button. The aria-expanded attribute on this button isn't handled correctly.

Also, there's one more inconsistency:

• The 'Close Menu' button does have an initial aria-expanded attribute which is printed out on the PHP side.
• The 'Close search' button does not.

We could argue whether a 'Close' button really needs an aria-expanded attribute. To me, a 'Close' button implies that something is open / expanded. Removing this aria-expanded attribute would also make things easier to fix on the JavaScript side.

The latest patch 53951.4.diff​

• Removes aria-expanded from the 'Close Menu' button.
• On the JavaScript side, skips both the 'Close search' and 'Close Menu' buttons by checking whether the attribute to toggle exists.

Some testing would be nice.

[alexstine]

@afercia Your patch works perfectly. None of the close buttons get aria-expanded anymore and attribute values seem to stay in sync.

Thanks!

[joedolson]

In 53051:

Twenty Twenty: Fix aria-expanded handling in search toggle.

Transform search into a dialog role and fix the handling of aria-expanded to synchronize mobile and desktop buttons.

Props utz119, alexstine, mukesh27, hareesh-pillai, sabernhardt, audrasjb, afercia.
Fixes #53951.