Skip to content

[iOS] Inherit AccessibilityTraits for views wrapped inside a WrapperView #27088

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

[iOS] Inherit AccessibilityTraits for views wrapped inside a WrapperView #27088

wants to merge 5 commits into from
+319 −1

Conversation

kubaflo
Copy link
Contributor

@kubaflo kubaflo commented Jan 13, 2025

Issues Fixed

Fixes #26990

<Border AutomationId="SaveButton"
          HeightRequest="100"
          Background="Red"
          WidthRequest="200"
          AutomationProperties.IsInAccessibleTree="True">
     <Border.GestureRecognizers>
          <TapGestureRecognizer Tapped="Button_Clicked"/>
     </Border.GestureRecognizers>
     <Border.Shadow>
          <Shadow Brush="Black"
                    Offset="0,3"
                    Radius="2"
                    Opacity="0.2"/>
     </Border.Shadow>
     <Label Text="Save"/>
</Border>
Before After

@kubaflo kubaflo requested a review from a team as a code owner January 13, 2025 00:32
@kubaflo kubaflo added platform/iOS 🍎 community ✨ Community Contribution t/a11y Relates to accessibility labels Jan 13, 2025
@jsuarezruiz
Copy link
Contributor

jsuarezruiz commented Jan 13, 2025 •
edited by jfversluis
Loading

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

jfversluis
jfversluis previously approved these changes Feb 25, 2025
View reviewed changes
@jfversluis

This comment was marked as off-topic.

Sign in to view
@jfversluis jfversluis added this to the .NET 9 SR5 milestone Feb 25, 2025
@jfversluis jfversluis self-assigned this Feb 25, 2025
@github-actions github-actions bot force-pushed the Accessibility-of-Border-control-breaks-when-Shadow-is-applied branch from a3c0ac1 to becc282 Compare February 25, 2025 09:45
@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jfversluis

This comment was marked as outdated.

Sign in to view
@github-actions github-actions bot force-pushed the Accessibility-of-Border-control-breaks-when-Shadow-is-applied branch from becc282 to 9abae1c Compare February 28, 2025 13:27
@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen modified the milestones: .NET 9 SR5, .NET 9 SR6 Mar 7, 2025
@github-actions github-actions bot force-pushed the Accessibility-of-Border-control-breaks-when-Shadow-is-applied branch from 9abae1c to 782b745 Compare March 21, 2025 11:53
@rmarinho
Copy link
Member

Can we add a test to inspect if the button has the correct Traits?

@dotnet dotnet deleted a comment from azure-pipelines bot Mar 21, 2025
@PureWeen PureWeen added the do-not-merge Don't merge this PR label Mar 24, 2025
@PureWeen PureWeen moved this from Todo to Ready To Review in MAUI SDK Ongoing Mar 24, 2025
jsuarezruiz
jsuarezruiz requested changes Mar 24, 2025
View reviewed changes
Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just, could include a device test verifying the AccessibilityTraits?

@github-project-automation github-project-automation bot moved this from Ready To Review to Changes Requested in MAUI SDK Ongoing Mar 24, 2025
@tj-devel709 tj-devel709 self-requested a review March 24, 2025 21:38
tj-devel709
tj-devel709 requested changes Mar 24, 2025
View reviewed changes
Copy link
Member

@tj-devel709 tj-devel709 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was working on something similar with the TGR on labels. Let me confirm some things before we merge this one

@jfversluis jfversluis added the area-controls-general General issues that span multiple controls, or common base classes such as View or Element label Mar 25, 2025
@tj-devel709
Copy link
Member

I'll try to get the UITests uploaded tomorrow

@kubaflo
Copy link
Contributor Author

kubaflo commented Mar 27, 2025

@tj-devel709 the test looks awesome

@tj-devel709
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@tj-devel709
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen modified the milestones: .NET 9 SR6, .NET 9 SR7 Apr 8, 2025
PureWeen
PureWeen requested changes Apr 14, 2025
View reviewed changes
Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might want to fix this issue higher up.

My guess is that we have differing places where we are applying accessibility properties to the WrapperView vs the PlatformView

So, I'm wondering if we just need to fix all those places to pick one.

For example, in the GesturePlatformManager.ios.cs we are applying all those values to the wrapper view but should we be applying them to the wrapped view?

Should we be applying these values to both?

I think it's risky putting this behavior purely in the WrapperView and will just lead to more missed scenarios down the road

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PureWeen PureWeen PureWeen requested changes

@jsuarezruiz jsuarezruiz jsuarezruiz requested changes

@tj-devel709 tj-devel709 tj-devel709 requested changes

@jfversluis jfversluis jfversluis left review comments

Requested changes must be addressed to merge this pull request.

Assignees

@jfversluis jfversluis

@tj-devel709 tj-devel709

Labels
area-controls-general General issues that span multiple controls, or common base classes such as View or Element community ✨ Community Contribution do-not-merge Don't merge this PR platform/iOS 🍎 t/a11y Relates to accessibility
Projects
MAUI SDK Ongoing
Status: Changes Requested
Milestone
.NET 9 SR7
Development

Successfully merging this pull request may close these issues.

[iOS] Accessibility of Border control breaks when Shadow is applied
6 participants
@kubaflo @jsuarezruiz @jfversluis @rmarinho @tj-devel709 @PureWeen