-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Interactivity API renders incorrect ARIA attributes for the Navigation block #54560
Comments
Thanks for raising the issue 🙂 Just to clarify, what I did in the PR you mentioned was replicate what is happening in the client. So I believe we should fix the JS logic as well. I'll try to fix that and submit a new PR. |
I had a quick look at the JS and as far as I can tell, it just sets correct and valid HTML attributes, when the dialog opens. |
Yes, but it wasn't removing the We've been triaging it and it seems it is a bug with Preact, which is used by the Interactivity API. I opened an issue in their repo for that. In the meantime, I opened this pull request with a workaround for that and fixing the issue reported here. |
I've merged the pull request that is supposed to fix this to ensure it is included in the next release. @afercia Please let me know if you think something else is needed, and I can work on it in a new pull request if needed. |
@SantosGuillamot thank you for the quick fix. The most important point is covered I think. Couple more things that I'd consider enhancements: 1
This produce a confusing announcement for screen reader users:
This announcement is a little confusing and unnecessarily noisy for screen reader users and ideally should be avoided. 2
is pointless because the component doesn't accept a |
Thanks for the detailed information! I didn't change that in the previous PR because those attributes had been there for a while, and I wasn't sure about the implications. I can work on a new PR to remove those attributes. |
I just created this pull request to handle that: link. |
Description
See #54343
Last week, #54343 merged into trunk some code for the Interactivity API Server Side Rendering. That change adds some default markup that I guess is meant to be changed dymanically via JavaScript. However, the default markup is incorrect.
I noticed whlle testing Twenty Twent-Four. so it is important to be aware the markup of the new default theme is invalid at the moment. Cc @carolinan
Quoting from #54343
These attributes are rendered by default in the desktop view when the navigation block is not inside a dialog, which is incorrect. Any Accessibility automated checker will flag this as an error.
<div>
element with no role (the role attribute is empty).Bad value for attribute role on element div
Screenshot of the current markup:
These attributes should be entirely omitted when the Navigation block is not rendered within a dialog. Not even sure why they are there but printing out markup that makes a page invalid and may impact assistive technology doesn't seem ideal.
@annezazu pinging you for considering to include this in the 6.4 board, when you have a chance) 🙏
Step-by-step reproduction instructions
role=""
aria-label="Menu"
aria-modal="false"
role="dialog"
aria-label="Menu"
aria-modal="ftrue"
Screenshots, screen recording, code snippet
No response
Environment info
No response
Please confirm that you have searched existing issues in the repo.
Yes
Please confirm that you have tested with all plugins deactivated except Gutenberg.
Yes
The text was updated successfully, but these errors were encountered: