Make dialog title a semantic heading element#51521
Make dialog title a semantic heading element#51521JamesFromIT wants to merge 4 commits intohome-assistant:devfrom
Conversation
There was a problem hiding this comment.
Hi @JamesFromIT
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the generic dialog title markup to use a semantic heading element to improve accessibility and assistive-technology navigation (per issue #3387).
Changes:
- Replace the dialog title wrapper from a
spanto anh1. - Add CSS to neutralize default heading styling for the
.titleclass.
| <h1 | ||
| class=${classMap({ title: true, alert: confirmPrompt })} | ||
| slot="title" | ||
| id="dialog-box-title" |
There was a problem hiding this comment.
Using an h1 inside dialogs can create multiple level-1 headings across the app, which can make heading navigation noisy for screen reader users. Consider using a lower-level heading (e.g., h2) for dialog titles, or switch to role=\"heading\" with an appropriate aria-level if you need to avoid hardcoding a global heading level.
There was a problem hiding this comment.
Interesting.
I've tested this implementation with the NVDA and JAWS screen readers and when I go to navigate by headers it seems to show only the dialog heading when one is open. Also, as expected, focus is trapped within the dialog until it is closed, so there's no way to manually navigate to the headings outside of the dialog using the arrow keys that I'm aware of. So, in this case, I don't see how this could generate noise.
I also think it's fair to say that the convention is probably well understood if it used in Deque University's Code Library of Accessibility Examples, W3C's ARIA Authoring Practices, and Harvard's Digital Accessibility Services. And, it is also what @steverep recommended in #3387 and he is a screen reader user himself.
That being said, if anyone has any strong opinions either way, I can change it.
As for using role and aria- attributes, use of existing native semantic elements (like h1, h2, etc.) where available is generally considered preferable. Using aria- attributes is useful in some cases, but because they can override accessibility behaviours, they can also cause confusion if implemented incorrectly. You also introduce an inconsistency risk as you then have to maintain this additional aria- code to make sure it still makes sense when the page structure changes.
Proposed change
Adds a semantic header tag to the dialog title in
dialog-box.ts. This is helpful for accessibility as some accessibility software uses semantic HTML to better understand the page.An example of an accessible dialog can be found in the Deque Code Library of Accessible Examples. This shows a
h1tag being used for the dialog title.This is one change in response to #3387.
Type of change
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
To help with the load of incoming pull requests: