Fixed presentation of checked / unchecked state for checkboxes and switches and selected state for radio buttons when used with a screen reader #4047
Conversation
…itches and selected state for radio buttons when used with a screen reader Signed-off-by: Peter Vágner <[email protected]>
|
|
|
Thank you for your contribution! Here are a few things to check in the PR to ensure it's reviewed as quickly as possible:
|
| role = Role.Checkbox, | ||
| enabled = true, | ||
| onValueChange = { onEnabledChanged(!state.isEnabled) } | ||
| ), |
There was a problem hiding this comment.
I am wondering if all these changes could be handle at a single place: in the ListItem implementation. It will reduce the number of change and will have the advantage of always use toggleable/selectable so that developer will not forget about it.
Here is a patch with a proposal:
ListItemModifier.patch
Note: not tested and may not always work, this patch is just to give some guidance about how it could be implemented. More work is necessary to make this patch an acceptable pull request.
There was a problem hiding this comment.
Hello, And thanks for the magic. I think it looks awesome. I am building the app with it applied and I'll test it in a little while.
There was a problem hiding this comment.
Hello again,
The patch is working as it should with no modifications from me.
It correctly applies modifiers as expected.
However I still have to go through the places I have modified and verify if the list item is really used in a way this implementation is expecting it.
For example:
- If there is an onClick handler on the switch or check box it-self it's being presented twice with some screen readers. Having click handler on both list item and switch / checkbox should therefore be avoided if we wish to make the screen reader experience fluent. Currently at least the favourite item in the room list long click menu is implemented like this.
- Some radio buttons are not working and I need to find out why for example when going to settings -> Advanced, there is a setting that affects app theme, it can be System, Light or Dark. And once the dialog is showing the radio buttons don't currently have the modifier applied as expected.
So if you are okay with the outcome and you do have enough bandwidth for doing it, please consider pushing your patch, in favour of this pull request and we should handle edge cases as follow up PRs. It's great improvement and it enables screen reader users to adjust settings on their own if they wish.
There was a problem hiding this comment.
Dear @bmarty
I have noticed you are leading efforts on improving accessibility these days.
I have found out what you are proposing for trailingContent is working fine however we should add the same thing for leadingContent on the ListItems to fix radio button presentation for screen reader users.
Can you please include this or would you like me to take your patch and edit this PR accordingly?
I'd appreciate if this got fixed in the mean time.
… be used only if the behavior is different than the `onClick` listener of the list item. It also has the effect to read twice the action when the screen reader is one. See #4047 (comment) for more details
|
Replaced by #4626 |
Remove duplication of onChange. As per the documentation, it has to be used only if the behavior is different than the onClick listener of the list item. It also has the effect to read twice the action when the screen reader is one. See #4047 (comment) for more details a11y: remove contentDescription on List item icon, else the text is read twice.
* a11y: add Modifier to improve accessibility of ListItems. Remove duplication of onChange. As per the documentation, it has to be used only if the behavior is different than the onClick listener of the list item. It also has the effect to read twice the action when the screen reader is one. See #4047 (comment) for more details a11y: remove contentDescription on List item icon, else the text is read twice. * Ensure that if the ListItem is not enabled, the trailing/leading content is also not enabled. * Update screenshots * Fix lint crash. --------- Co-authored-by: ElementBot <[email protected]>
Content
Checkboxes, switches and radio buttons are implemented in a way that icon, title, the interactive control and optional description is encapsulated in a parent composable which receives onClick event. Checkboxes, switches and radio buttons them selves have no click handler. When considering universal user experience it's very nice approach as it increases touch target for these UI controls. However when considering screen reader accessibility screen readers are not receiving role and state information for these controls. Thus when using the app with a screen reader I can't find out if a check box is checked or not, if a switch is turned on or not and which of the radio buttons is selected.
I am trying to address this by adding toggleable and selectable modifiers where appropriate.
Motivation and context
I am using this app daily, I used to experiment and guess the state of these checkboxes, switches and radio buttons, ask my friends until I have found I can try to improve it.
Screenshots / GIFs
I believe appearance is unchanged. Also I have opted not to create and work with screenshots at all since I am unable to.
Tests
Tested on a real device.
Tested devices
Checklist