Remove not needed aria-hidden attribute in navigation block experiment #51044
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SantosGuillamot
requested review from
spacedmonkey and
tellthemachines
as code owners
SantosGuillamot
requested review from
jeryj,
luisherranz,
DAreRodz and
cbravobernal
and removed request for
spacedmonkey and
tellthemachines
SantosGuillamot
added
[Type] Bug
SantosGuillamot
changed the title
|
Size Change: +192 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
luisherranz
reviewed
May 29, 2023
|
@SantosGuillamot Correct. The only time Hope this helps. |
It totally does! Thanks a lot for the information Taking a look at the styles, it seems that it is adding |
SantosGuillamot
changed the title
luisherranz
approved these changes
May 30, 2023
SantosGuillamot
deleted the
fix/aria-hidden-navigation-block-interactivity-api
branch
sethrubenstein
pushed a commit
to pewresearch/gutenberg
that referenced
this pull request
Jul 13, 2023
WordPress#51044) * Add aria-hidden selector * Use `offsetParent` to check if the menu is visible * Remove `aria-hidden` attribute
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
As reported here: In the version of the navigation block using the Interactivity API, in some cases we are defining
aria-hidden: trueeven for the desktop version, which is not correct. This pull request aims to solve that.Why?
It causes issues with screen readers in the desktop version, when the elements are shown but
aria-hiddenis true.How?
I added a selector that reads:If the menu is open ->aria-hidden: false.If the menu is not open -> It checks if the "Open menu" is visible. If it is,aria-hidden: true. If it isn'taria-hidden: false.I couldn't find any other way of checking this because, if I am not mistaken, it is handled with CSS.I finally removed the aria-hidden attribute directly because it seems that it shouldn't be used in this use case, where the element is hidden using
display: none, according to MDN docs: "aria-hidden="true" should not be added when the element or the element's ancestor is hidden with display: none".Testing Instructions
aria-hiddeninside thewp-block-navigation__responsive-containeris true when the menu is wrapped, and false when the menu opens.aria-hiddenis always false.Screenshots or screencast
Desktop version
Mobile version
