-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Replace ha-clickable-list-item with native material 3 components #20920
Conversation
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughWalkthroughThe recent changes focus on updating various components within the Home Assistant frontend to use new list and menu components. Specifically, Changes
Assessment against linked issues
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional context usedBiome
Additional comments not posted (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems nice 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made a small import correction, but still reviewing for a11y. This is my first detailed look at the M3 lists and it seems they've solved some a11y issues but introduced others. 😞
Co-authored-by: Steve Repsher <[email protected]>
This comment was marked as off-topic.
This comment was marked as off-topic.
Could you clarify what changes you noticed? This PR isn't really focused on accessibility, but if makes things worse, I'm not really going forward with it. |
Menu items no longer activating on space key is a significant regression, especially since it sort of appears to activate by closing the menu. I don't know how they could miss such basic a11y there. The static lists have ARIA that is just plain wrong (native anchors with a non-interactive I've briefly tried messing with the attributes to fix but not having success. I need to look at their code and probably file some issues if they don't exist already. |
The spacebar thing is something that has never been a part of the material spec, I think we added it in as custom logic: I will set the "interactive" on the lists where possible and see if I can make it have the right ARIA roles. It's also something we can set as an attribute as far as I'm aware. |
Material is irrelevant; it is part of W3C guidelines and best practices.
The |
I've had a look at bringing this back to undrafting but the accessilbility issues are not easy to fix since I'm not sure where I can read about those guidelines. I found that spacebar should expand or unexpand the list of a menu button, which it does. At this time we're sure that accessibility is unacceptable with the current m3 implementation? How is the current implementation accessible of the ha-clickable-list-item component or are there problems with that implementation as well? |
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
Proposed change
Replace ha-clickable-list-item, either with a ha-list-item-new if embedded within a list or a ha-menu-item if embedded within a menu. I decided to remove the component since it's a normal thing in the new material components. It's a drop in replacement although the link will no longer trigger if you activate with a spacebar.
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
Summary by CodeRabbit
mwc-list
andmwc-list-item
withha-list-new
andha-list-item-new
.ha-clickable-list-item
withha-menu-item
andha-button-menu
withha-button-menu-new
.These changes enhance the user interface by standardizing components across different configuration panels.