Skip to content
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

Update semantics #1039

Merged
merged 28 commits into from
Jun 10, 2021
Merged

Update semantics #1039

merged 28 commits into from
Jun 10, 2021

Conversation

rachelkang
Copy link
Member

@rachelkang rachelkang commented May 17, 2021

Description of Change

  • Update semantics page to address differences between iOS and Android
  • Make controls that have semantics also be screen reader accessible by default
  • On Android: Use AccessibilityDelegate info.Text instead of contentDescription
  • On Android: Make EditText description get read aloud, consistent with iOS and helpful for LabeledBy alternative

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)
  • Targets a single property for a single control (or intertwined few properties)
  • Adds the property to the appropriate interface
  • Avoids any changes not essential to the handler property
  • Adds the mapping to the PropertyMapper in the handler
  • Adds the mapping method to the Android, iOS, and Standard aspects of the handler
  • Implements the actual property updates (usually in extension methods in the Platform section of Core)
  • Tags ported renderer methods with [PortHandler]
  • Adds an example of the property to the sample project (MainPage)
  • Adds the property to the stub class
  • Implements basic property tests in DeviceTests

Does this PR touch anything that might affect accessibility?

  • Does this PR introduce a new control? (If yes, add an example using SemanticProperties to the SemanticsPage)
  • APIs that modify focusability?
  • APIs that modify any text property on a control?
  • Does this PR modify view nesting or view arrangement in anyway?
  • Is there the smallest possibility that your PR will change accessibility?
  • I'm not sure, please help me

If any of the above checkboxes apply to your PR, then the PR will need to provide testing to demonstrate that accessibility still works.

@rachelkang rachelkang requested a review from PureWeen May 17, 2021 19:44
@@ -55,18 +55,18 @@
SemanticProperties.Hint="Hint text"/>

<Entry
Text="Entry text TH"
Text="Entry text, iOS:DTH, Android:TH"
Copy link
Member

Choose a reason for hiding this comment

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

I wonder how these read on XCT?

Like if we use the AccessibilityDelegate instead of the content description

Copy link
Member Author

Choose a reason for hiding this comment

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

It reads the same on XCT at the moment

@@ -83,6 +83,8 @@ public static void UpdateSemantics(this UIView nativeView, IView view)

if (semantics == null)
return;
else
nativeView.IsAccessibilityElement = true;
Copy link
Member

Choose a reason for hiding this comment

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

These should only get set if there's an actual value set for Description/Hint

Copy link
Member Author

Choose a reason for hiding this comment

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

fixing now - but can you remind me why the null check is sufficient for Android and not iOS?

Copy link
Member

Choose a reason for hiding this comment

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

That statement applies to both Android and iOS

Copy link
Member Author

Choose a reason for hiding this comment

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

gotcha - will fix both here and in XCT soon

@dotnet dotnet deleted a comment May 24, 2021
@dotnet dotnet deleted a comment May 24, 2021
@dotnet dotnet deleted a comment May 24, 2021
@dotnet dotnet deleted a comment May 24, 2021
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.

Based on our conversations we need to fix Android to read out like iOS with the Entry control

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.

Add a layout example so we can see the behaviors on a layout

<stacklayout SemanticProperty.Description="Hello">
<label
<button
<stacklayout SemanticProperty.Description="Hello">
<label
<label

@rachelkang rachelkang changed the title Update semantics page Update semantics Jun 9, 2021
[Fact(DisplayName = "Semantic Description is set correctly")]
[Fact(DisplayName = "Semantic Description is set correctly"
#if MONOANDROID
, Skip = "This value can't be validated through automated tests"
Copy link
Member

Choose a reason for hiding this comment

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

Because we switched to using delegates for reading out the semantic description we can no longer verify these values through unit tests

@PureWeen PureWeen merged commit c1f8982 into main Jun 10, 2021
@PureWeen PureWeen deleted the audit-semantics-page branch June 10, 2021 19:21
@rachelkang rachelkang added the legacy-area-a11y Relates to accessibility label Aug 9, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Dec 26, 2023
@Eilon Eilon added the t/a11y Relates to accessibility label May 13, 2024
@samhouts samhouts added the fixed-in-6.0.100-preview.5 Look for this fix in 6.0.100-preview.5! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
fixed-in-6.0.100-preview.5 Look for this fix in 6.0.100-preview.5! legacy-area-a11y Relates to accessibility t/a11y Relates to accessibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants