-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[Accessibility] Add Accessibility Traits to CollectionView items on iOS/Catalyst #27971
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
Conversation
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.
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
src/Controls/src/Core/Handlers/Items/iOS/SelectableItemsViewController.cs
Outdated
Show resolved
Hide resolved
7205a86
to
298c6c1
Compare
This feels tight to me, but... we are forcing some a11y on the platform that never had it. How are selectable items in native iOS? Just wrong and all apps do this, or are we breakingsomething or not setting something somewhere to auto enable a11y? |
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.
Code looks good and TJ's logic is sound.
I am wondering wny iOS needs this cusotm code for a11y - either iOS is broken, or we broke it. But, I have been binging, googling and AI-ing and many sources are saying that we should be setting cell.isAccessibilityElement
and also just set the traits to indicate that it is tappable.
So, based on that and the fact that TJ knows what he is doing and I do not... Approved!
Yeah this is a good question. So from testing other first party apps on Catalyst, other items inside a collectionview usually come up as a button or sometimes a group inside the accessibility inspector. Note the type "button" in the "basic" section of the Accessibility inspector on the right. I found some other examples of apps that do not have the "button" as the type but have custom options such as this one here in the news app: The reason this is important is that when a user is using the screenreader, they need to know what to do with the item that is focused. When the type is set to button, it reads out a message such as "Tj's super cool thing, button, double tap to activate" or some keyboard combination to press to enter the group and select the button. Prior to this PR, the items in a collectionview when the selectionMode is Single or Multiple, would not let the user know that those elements are selectable. Setting the SemanticProperties.Description marks the item as isAccessibilityElement and allows us to give the items inside the collectionview a description to be read by the screenreader, but still doesn't give it a type to show what action to do. So the isAccessibilityElement is just not sufficient enough for these purposes. The workaround was to put an empty tapgestureRecognizer inside the collectionview item and then add InputTransparent=true on the container of the item so it doesn't eat the tap but this makes it so we don't need to do that anymore. |
/rebase |
5071f77
to
40fddfe
Compare
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.
Should we create tests with my complex templates ? what if I have a Grid with a StackLayout and Datepicker ? will it find the UIControl
This will just look for the outerlayer that is just below the DataTemplate and only inside a collectionview with the selectionmode set to single or multiple. If you have the CollectionView -> ItemTemplate -> DataTemplate -> Grid -> StackLayout -> DatePicker, this PR will just try to add the the button announcement to the Grid element. It will also not mark anything as a button that is a UIControl. So if you had CollectionView -> ItemTemplate -> DataTemplate -> DatePicker, it will not add the button announcement to the DatePicker since that would be confusing. With that said, it probably wouldn't hurt to have a more complex layout in the testing. Perhaps these two scenarios I just mentioned. |
Description of Change
This change allows the items used in CV1 and CV2 to have the accessibility trait of Button when the SelectionMode is set to Single or Multiple. This is important because the Screenreaders in iOS and Mac Catalyst will be able to alert the user that the items act as buttons and are selectable.
Issues Fixed
Partially fixes (but do not close): #26817