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

Add accessibility identifier to SemanticsProperties #138331

Merged

Conversation

bartekpacia
Copy link
Member

@bartekpacia bartekpacia commented Nov 12, 2023

This PR adds String? identifier to Semantics and SemanticsProperties. The identifier will be exposed on Android as resource-id and on iOS as accessibilityIdentifier.

Mainly targeted at #17988

Initial Engine PR with Android support: flutter/engine#47961
iOS Engine PR: flutter/engine#48858

Migration

This change breaks the SemanticsUpdateBuilder API which is on the Framework<-->Engine border. For more details see engine PR.

Steps:
part 1: [engine] add SemanticsUpdateBuilderNew flutter/engine#47961
part 2: [flutter] use SemanticsUpdateBuilderNew <-- we are here
part 3: [engine] update SemanticsUpdateBuilder to be the same as SemanticsUpdateBuilderNew flutter/engine#48882
part 4: [flutter] use (now updated) SemanticsUpdateBuilder again #139942
part 5: [engine] remove SemanticsBuilderNew flutter/engine#49139

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) labels Nov 12, 2023
Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

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

which accessibilitynodeinfo property would this new identifier set to? There doesn't seem to be a resourceId in the accessibilitynodeinfo https://developer.android.com/reference/android/view/accessibility/AccessibilityNodeInfo

Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

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

ah it is viewIdResourceName just saw the engine pr

packages/flutter/lib/src/semantics/semantics.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/semantics/semantics.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/semantics/semantics.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/semantics/semantics.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/semantics/semantics.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/semantics/semantics.dart Outdated Show resolved Hide resolved
@@ -4188,6 +4219,15 @@ class SemanticsConfiguration {
}
}

/// An identifier of the owning [RenderObject] used for UI testing. This value
Copy link
Contributor

Choose a reason for hiding this comment

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

This doc seems a bit confusing. The UIAutomator interact with semantics node, not renderObject.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, thank you. I'm still new to this code. Will fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here I took inspiration from other SemanticsConfiguration fields, e.g.:

/// A textual description of the owning [RenderObject].
///
/// Setting this attribute will override the [attributedLabel].
///
/// The reading direction is given by [textDirection].
///
/// See also:
///
/// * [attributedLabel], which is the [AttributedString] of this property.
String get label => _attributedLabel.string;

What'd you change?

packages/flutter/lib/src/semantics/semantics.dart Outdated Show resolved Hide resolved
@bartekpacia bartekpacia force-pushed the feature/accessibility_identifier branch 2 times, most recently from 0f9f861 to aed764f Compare November 30, 2023 20:30
Copy link
Contributor

@chunhtai chunhtai 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 some nit picking

packages/flutter/lib/src/semantics/semantics.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/semantics/semantics.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/semantics/semantics.dart Outdated Show resolved Hide resolved
@bartekpacia
Copy link
Member Author

Thank you - I applied your suggestions.

auto-submit bot pushed a commit to flutter/engine that referenced this pull request Dec 4, 2023
…ndroid (#47961)

Accompanying framework PR: flutter/flutter#138331

This PR implements support for exposing `SemanticsProperties.identifier` on Android as `resource-id`. Mainly targeted at flutter/flutter#17988. Would also fix https://github.com/flutter/flutter/issues/issues/137735 but it was marked as duplicate. Anyway, there's a lot of context in that issue.

This PR requires changing the `SemanticsUpdateBuilder` interface (defined in engine) that framework depends on, so it requires introducing a temporary API ([see question I asked on Discord](https://discord.com/channels/608014603317936148/608018585025118217/1174845658033819729) to learn more about this approach).

Steps:
**part 1: [engine] add `SemanticsUpdateBuilderNew`** <-- we are here
part 2: [flutter] use `SemanticsUpdateBuilderNew`
part 3: [engine] update `SemanticsUpdateBuilder` to be the same as `SemanticsUpdateBuilderNew`*
part 4: [flutter] use (now updated) `SemanticsUpdateBuilder` again.
part 5: [engine] remove `SemanticsBuilderNew`

I'd like to do these changes first, and only then continue with [the proper framework PR](flutter/flutter#138331).

*More specifically: update `SemanticsUpdateBuilder.updateNode()` to be the same as `SemanticsUpdateBuilderNew.updateNode()`. Number of arguments that function takes is the only change.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
@bartekpacia bartekpacia force-pushed the feature/accessibility_identifier branch 2 times, most recently from 317068a to 95250ed Compare December 4, 2023 23:39
@bartekpacia bartekpacia marked this pull request as ready for review December 5, 2023 08:41
@bartekpacia bartekpacia force-pushed the feature/accessibility_identifier branch from 95250ed to 252da73 Compare December 6, 2023 09:11
@github-actions github-actions bot added the a: tests "flutter test", flutter_test, or one of our tests label Dec 6, 2023
@bartekpacia bartekpacia force-pushed the feature/accessibility_identifier branch 2 times, most recently from 36bcac5 to 3cee5ab Compare December 8, 2023 15:34
@bartekpacia bartekpacia force-pushed the feature/accessibility_identifier branch from 3cee5ab to 713c47b Compare December 9, 2023 19:50
@srawlins
Copy link
Contributor

srawlins commented Dec 9, 2023

@bartekpacia the latest "Linux customer_testing" bot failure says it grabbed devtools commit 914356dedc97b4bafc33cc3f9c0334fefbb3417f. Not sure why...

@bartekpacia
Copy link
Member Author

Yeah, indeed. At first I thought the commit is maybe pinned elsewhere but nope.

@bartekpacia bartekpacia force-pushed the feature/accessibility_identifier branch from 687b33d to a2083fb Compare December 11, 2023 14:24
@bartekpacia bartekpacia added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 11, 2023
@auto-submit auto-submit bot merged commit 948523b into flutter:master Dec 11, 2023
67 checks passed
@bartekpacia bartekpacia deleted the feature/accessibility_identifier branch December 11, 2023 18:04
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2023
auto-submit bot pushed a commit to flutter/engine that referenced this pull request Dec 11, 2023
This PR adds `String? identifier` to `SemanticsUpdateBuilder` (currently it's only available in the temproary `SemanticsUpdateBuilderNew` API.

This is mainly targeted at flutter/flutter#17988

Steps:
part 1: [engine] add `SemanticsUpdateBuilderNew` #47961
part 2: [flutter] use `SemanticsUpdateBuilderNew`  flutter/flutter#138331
**part 3: [engine] update `SemanticsUpdateBuilder` to be the same as `SemanticsUpdateBuilderNew`** <-- we are here
part 4: [flutter] use (now updated) `SemanticsUpdateBuilder` again.
part 5: [engine] remove `SemanticsBuilderNew`
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Dec 12, 2023
…5648)

Manual roll requested by dit@google.com

flutter/flutter@c642f4e...9719097

2023-12-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 5587d26aa2d4 to 4c309195b79d (1 revision) (flutter/flutter#139936)
2023-12-11 rmolivares@renzo-olivares.dev Fix SelectionArea select-word edge cases (flutter/flutter#136920)
2023-12-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9b85b76db0de to 5587d26aa2d4 (3 revisions) (flutter/flutter#139933)
2023-12-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7eb6b7cab60c to 9b85b76db0de (2 revisions) (flutter/flutter#139931)
2023-12-11 31859944+LongCatIsLooong@users.noreply.github.com Use dart analyze package for `num.clamp` (flutter/flutter#139867)
2023-12-11 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#139926)
2023-12-11 jhy03261997@gmail.com Handle the case when _CupertinoBackGestureDetector is disposed during the drag.  (flutter/flutter#139585)
2023-12-11 barpac02@gmail.com Add accessibility identifier to `SemanticsProperties` (flutter/flutter#138331)
2023-12-11 38378650+hgraceb@users.noreply.github.com Improve slider's value indicator display test (flutter/flutter#139198)
2023-12-11 mateusfccp@gmail.com Add `enabled` property to `ExpansionTile` (flutter/flutter#139519)
2023-12-11 engine-flutter-autoroll@skia.org Roll Packages from 6cd0657 to cb6dbcd (9 revisions) (flutter/flutter#139911)
2023-12-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from bc0222b64c96 to 7eb6b7cab60c (1 revision) (flutter/flutter#139891)
2023-12-10 engine-flutter-autoroll@skia.org Roll Flutter Engine from fb80aafd259b to bc0222b64c96 (1 revision) (flutter/flutter#139885)
2023-12-09 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#139864)
2023-12-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from b75960a5820a to fb80aafd259b (1 revision) (flutter/flutter#139863)
2023-12-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from e80c090d09c6 to b75960a5820a (1 revision) (flutter/flutter#139853)
2023-12-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from 101396fd3b82 to e80c090d09c6 (2 revisions) (flutter/flutter#139851)
2023-12-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from 503584615fd7 to 101396fd3b82 (2 revisions) (flutter/flutter#139847)
2023-12-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from e9cb19fa637a to 503584615fd7 (1 revision) (flutter/flutter#139837)
2023-12-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7dc51b85a634 to e9cb19fa637a (1 revision) (flutter/flutter#139831)
2023-12-08 xilaizhang@google.com [flutter release] Add cherry pick template for pull request description (flutter/flutter#139590)
2023-12-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from 03c5f016e919 to 7dc51b85a634 (1 revision) (flutter/flutter#139829)
2023-12-08 goderbauer@google.com Add Overlay.wrap for convenience (flutter/flutter#139823)
2023-12-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from 72da960e2ef2 to 03c5f016e919 (1 revision) (flutter/flutter#139826)
2023-12-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from 5dd2619c282b to 72da960e2ef2 (1 revision) (flutter/flutter#139821)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC dit@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
HugoOlthof pushed a commit to moneybird/packages that referenced this pull request Dec 13, 2023
…lutter#5648)

Manual roll requested by dit@google.com

flutter/flutter@c642f4e...9719097

2023-12-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 5587d26aa2d4 to 4c309195b79d (1 revision) (flutter/flutter#139936)
2023-12-11 rmolivares@renzo-olivares.dev Fix SelectionArea select-word edge cases (flutter/flutter#136920)
2023-12-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9b85b76db0de to 5587d26aa2d4 (3 revisions) (flutter/flutter#139933)
2023-12-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7eb6b7cab60c to 9b85b76db0de (2 revisions) (flutter/flutter#139931)
2023-12-11 31859944+LongCatIsLooong@users.noreply.github.com Use dart analyze package for `num.clamp` (flutter/flutter#139867)
2023-12-11 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#139926)
2023-12-11 jhy03261997@gmail.com Handle the case when _CupertinoBackGestureDetector is disposed during the drag.  (flutter/flutter#139585)
2023-12-11 barpac02@gmail.com Add accessibility identifier to `SemanticsProperties` (flutter/flutter#138331)
2023-12-11 38378650+hgraceb@users.noreply.github.com Improve slider's value indicator display test (flutter/flutter#139198)
2023-12-11 mateusfccp@gmail.com Add `enabled` property to `ExpansionTile` (flutter/flutter#139519)
2023-12-11 engine-flutter-autoroll@skia.org Roll Packages from 6cd0657 to cb6dbcd (9 revisions) (flutter/flutter#139911)
2023-12-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from bc0222b64c96 to 7eb6b7cab60c (1 revision) (flutter/flutter#139891)
2023-12-10 engine-flutter-autoroll@skia.org Roll Flutter Engine from fb80aafd259b to bc0222b64c96 (1 revision) (flutter/flutter#139885)
2023-12-09 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#139864)
2023-12-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from b75960a5820a to fb80aafd259b (1 revision) (flutter/flutter#139863)
2023-12-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from e80c090d09c6 to b75960a5820a (1 revision) (flutter/flutter#139853)
2023-12-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from 101396fd3b82 to e80c090d09c6 (2 revisions) (flutter/flutter#139851)
2023-12-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from 503584615fd7 to 101396fd3b82 (2 revisions) (flutter/flutter#139847)
2023-12-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from e9cb19fa637a to 503584615fd7 (1 revision) (flutter/flutter#139837)
2023-12-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7dc51b85a634 to e9cb19fa637a (1 revision) (flutter/flutter#139831)
2023-12-08 xilaizhang@google.com [flutter release] Add cherry pick template for pull request description (flutter/flutter#139590)
2023-12-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from 03c5f016e919 to 7dc51b85a634 (1 revision) (flutter/flutter#139829)
2023-12-08 goderbauer@google.com Add Overlay.wrap for convenience (flutter/flutter#139823)
2023-12-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from 72da960e2ef2 to 03c5f016e919 (1 revision) (flutter/flutter#139826)
2023-12-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from 5dd2619c282b to 72da960e2ef2 (1 revision) (flutter/flutter#139821)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC dit@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
auto-submit bot pushed a commit that referenced this pull request Dec 16, 2023
…y `SemanticsUpdateBuilderNew` (#139942)

This PR removes all usages of the temporary `SemanticsUpdateBuilderNew` API in favor of `SemanticsUpdateBuilder`. These two APIs are the same as of now.

This is mainly targeted at #17988

Steps:
part 1: [engine] add `SemanticsUpdateBuilderNew` flutter/engine#47961
part 2: [flutter] use `SemanticsUpdateBuilderNew` #138331
part 3: [engine] update `SemanticsUpdateBuilder` to be the same as `SemanticsUpdateBuilderNew` flutter/engine#48882
**part 4: [flutter] use (now updated) `SemanticsUpdateBuilder` again** <-- we are here
part 5: [engine] remove `SemanticsBuilderNew`
auto-submit bot pushed a commit to flutter/engine that referenced this pull request Dec 18, 2023
)

This PR completest the migration by removing `SemanticsUpdateBuilderNew` from the engine.

This is mainly targeted at flutter/flutter#17988

Steps:
part 1: [engine] add `SemanticsUpdateBuilderNew` #47961
part 2: [flutter] use `SemanticsUpdateBuilderNew` flutter/flutter#138331
part 3: [engine] update `SemanticsUpdateBuilder` to be the same as `SemanticsUpdateBuilderNew` #48882
part 4: [flutter] use (now updated) `SemanticsUpdateBuilder` again flutter/flutter#139942
**part 5: [engine] remove `SemanticsBuilderNew`** <-- we are here
Michal-MK pushed a commit to Michal-MK/flutter that referenced this pull request Jan 8, 2024
This PR adds `String? identifier` to `Semantics` and `SemanticsProperties`. The `identifier` will be exposed on Android as `resource-id` and on iOS as `accessibilityIdentifier`.

Mainly targeted at flutter#17988

Initial Engine PR with Android support: flutter/engine#47961
iOS Engine PR: flutter/engine#48858

### Migration

This change breaks the SemanticsUpdateBuilder API which is on the Framework<-->Engine border. For more details see [engine PR](flutter/engine#47961).

Steps:
part 1: [engine] add `SemanticsUpdateBuilderNew` flutter/engine#47961
**part 2: [flutter] use `SemanticsUpdateBuilderNew`**  <-- we are here
part 3: [engine] update `SemanticsUpdateBuilder` to be the same as `SemanticsUpdateBuilderNew`*
part 4: [flutter] use (now updated) `SemanticsUpdateBuilder` again.
part 5: [engine] remove `SemanticsBuilderNew`
Michal-MK pushed a commit to Michal-MK/flutter that referenced this pull request Jan 8, 2024
…y `SemanticsUpdateBuilderNew` (flutter#139942)

This PR removes all usages of the temporary `SemanticsUpdateBuilderNew` API in favor of `SemanticsUpdateBuilder`. These two APIs are the same as of now.

This is mainly targeted at flutter#17988

Steps:
part 1: [engine] add `SemanticsUpdateBuilderNew` flutter/engine#47961
part 2: [flutter] use `SemanticsUpdateBuilderNew` flutter#138331
part 3: [engine] update `SemanticsUpdateBuilder` to be the same as `SemanticsUpdateBuilderNew` flutter/engine#48882
**part 4: [flutter] use (now updated) `SemanticsUpdateBuilder` again** <-- we are here
part 5: [engine] remove `SemanticsBuilderNew`
Markzipan pushed a commit to Markzipan/flutter that referenced this pull request Jan 9, 2024
This PR adds `String? identifier` to `Semantics` and `SemanticsProperties`. The `identifier` will be exposed on Android as `resource-id` and on iOS as `accessibilityIdentifier`.

Mainly targeted at flutter#17988

Initial Engine PR with Android support: flutter/engine#47961
iOS Engine PR: flutter/engine#48858

### Migration

This change breaks the SemanticsUpdateBuilder API which is on the Framework<-->Engine border. For more details see [engine PR](flutter/engine#47961).

Steps:
part 1: [engine] add `SemanticsUpdateBuilderNew` flutter/engine#47961
**part 2: [flutter] use `SemanticsUpdateBuilderNew`**  <-- we are here
part 3: [engine] update `SemanticsUpdateBuilder` to be the same as `SemanticsUpdateBuilderNew`*
part 4: [flutter] use (now updated) `SemanticsUpdateBuilder` again.
part 5: [engine] remove `SemanticsBuilderNew`
Markzipan pushed a commit to Markzipan/flutter that referenced this pull request Jan 9, 2024
…y `SemanticsUpdateBuilderNew` (flutter#139942)

This PR removes all usages of the temporary `SemanticsUpdateBuilderNew` API in favor of `SemanticsUpdateBuilder`. These two APIs are the same as of now.

This is mainly targeted at flutter#17988

Steps:
part 1: [engine] add `SemanticsUpdateBuilderNew` flutter/engine#47961
part 2: [flutter] use `SemanticsUpdateBuilderNew` flutter#138331
part 3: [engine] update `SemanticsUpdateBuilder` to be the same as `SemanticsUpdateBuilderNew` flutter/engine#48882
**part 4: [flutter] use (now updated) `SemanticsUpdateBuilder` again** <-- we are here
part 5: [engine] remove `SemanticsBuilderNew`
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: tests "flutter test", flutter_test, or one of our tests autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants