-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Setting correct roles for lists with check/radio options #28684
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
base: dev
Are you sure you want to change the base?
Setting correct roles for lists with check/radio options #28684
Conversation
| constructor() { | ||
| super(); | ||
| if (this.innerRole == null) this.innerRole = "list"; | ||
| if (this.itemRoles == null) this.itemRoles = "listitem"; | ||
| } |
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.
Don't set default values like this, but use:
@property() public innerRole: string | null = "list";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.
This might be a good idea, but I view setting a default like this as dangerous and out of scope for just fixing menus with radios/checkboxes and remove it from this PR. There are just too many lists this would affect and impossible to test them all.
| ></search-input> | ||
| </ha-dialog-header> | ||
| <ha-list multi> | ||
| <ha-list multi innerRole="menu" itemRoles="menuitemcheckbox"> |
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.
Did you test if this works? As the child of ha-list is lit-virtualizer my guess is that the items in the virtualizer are not targeted by this.
|
Is there anyone that has much experience with |
|
@uptimeZERO there is a merge conflict |
|
@bramkragten I can make time to pull this branch and test it, but not for several days at least given the holidays. @uptimeZERO if you tested this with a screen reader/OS, please describe. These roles are difficult to get right and I think this should be tested thoroughly. Thank you for the effort and I'll do my best to help. |
Proposed change
Updating
ha-listusages to set the correct role when used as a checklist/radiolist and also setting default role.Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: