-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Added semanticsIdentifier to Text Widgets
#163843
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
Added semanticsIdentifier to Text Widgets
#163843
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
@chunhtai @Renzo-Olivares I wonder if you guys have thoughts on this semantics change for Text. |
| @@ -0,0 +1,165 @@ | |||
| // Copyright 2014 The Flutter Authors. All rights reserved. | |||
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.
Can you put this in examples/api/lib/widgets ?
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.
Moved it
| child: Text.rich( | ||
| TextSpan( | ||
| text: 'Please open the ', | ||
| // semanticsIdentifier: 'please_open', |
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.
why do we comment these out?
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.
I forgot to restore after creating Appium screenshots. Fixed it now.
| for (final InlineSpanSemanticsInformation info in infoList) { | ||
| if (info.requiresOwnNode) { | ||
| combined.add( | ||
| InlineSpanSemanticsInformation( |
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.
I am not sure how this was working before, if the first one in the infoList, won't we add an empty info here?
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.
You're right, there is no need to make changes to this method. It is working fine without it.
| ); | ||
| } | ||
| workingLabel += effectiveLabel; | ||
| workingIdentifier = info.semanticsIdentifier; |
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.
This should always be null according to the doc, right? can you use assert instead?
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.
You're right, there is no need to make changes to this method. It is working fine without it.
| } | ||
| } | ||
| combined.add( | ||
| InlineSpanSemanticsInformation( |
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.
This is also some weird code, wouldn't we added am empty one if last one requiresOwnNode
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.
You're right, there is no need to make changes to this method. It is working fine without it.
| final List<InlineSpanSemanticsInformation> collector = <InlineSpanSemanticsInformation>[]; | ||
| const TextSpan(text: 'aaa', semanticsLabel: 'bbb').computeSemanticsInformation(collector); | ||
| const TextSpan( | ||
| text: 'aaa', |
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.
Can you add a test for text widget with textspan that has semantics identifier and assert a semantics node with that identifier is created?
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.
Added a test to test/widgets/text_semantics_test.dart
|
looks like there is a formatting issue |
chunhtai
left a comment
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.
LGTM, except one comment
| @override | ||
| void initState() { | ||
| super.initState(); | ||
| WidgetsBinding.instance.addPostFrameCallback((_) { |
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.
This seems to be debug code, can you remove this?
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.
Done
|
Hi @chunhtai, I have addressed the remaining issues. Please check. Thanks! |
Renzo-Olivares
left a comment
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.
LGTM w/ small comment. Thank you for your contribution!
|
|
||
| /// True if this configuration should get its own semantics node. | ||
| /// | ||
| /// This will be the case of the [recognizer] is not null, of if |
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.
nit: this paragraph should be: This will be the case if the [recognizer] is not null, or if [isPlaceholder] is true, or if [semanticsIdentifier] has a value.
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.
Fixed it, thank you!
a7aedff to
9fe5e62
Compare
|
Hi @chunhtai | @Renzo-Olivares , I see the "Google testing" workflow is failing. I am not sure how to resolve it or proceed with the merge. This is my first time contributing to flutter so please advise on how to merge it. Thanks! |
|
This breaks some internal code where it implements the abstract interface InlineSpanSemanticsInformation, the internal test will need to be migrated |
|
waiting for cl/734306543 |
|
autosubmit label was removed for flutter/flutter/163843, because - The status or check suite Mac_arm64 build_tests_3_4 has failed. Please fix the issues identified (or deflake) before re-applying this label. |
This PR aims to add `semanticsIdentifier` to `Text` and some of its internal objects to pass the semantics information for adding identifier to the semantics nodes From the issue filed at flutter#163842, the following is a description of the problem. The [semantics identifier](https://api.flutter.dev/flutter/semantics/SemanticsData/identifier.html) helps in uniquely identifying elements using UI automation tools like Appium, UIAutomator and XCUITests by setting identifiers that the screen readers cannot see but the said tools can. This is especially useful when working with a multi-lingual or multi-tenant app, where the element IDs need to be unique but the content can be different. The `Semantics` widget already has support for declaring it. However, the `Text` and `Text.rich` variants only support setting `semanticsLabel` without explicitly setting the identifiers. The widgets themselves can be wrapped with a `Semantics` widget but it still does not cater for a rich text that can have multiple text spans, each containing unique lables and identifiers, and optionally gesture detectors for handling links. Consider the following UI for two different tenants: <img width="229" alt="Image" src="https://github.com/user-attachments/assets/e8a24588-d94d-42fc-ba6c-ce39959207ae" /> Here, both the tenants utilise different strings to convey the same message. The structure of the message stays the same so the identifiers help in unifying the element identification process across the tenant apps in the automation tools without having to write another script for every other tenant. Without the identifiers, the automation scripts require a rewrite per tenant to be able to successfully locate the element and even tap on the hyperlink. # With PR Changes ## Appium Views For the given sample code, <details><summary>Text.rich Sample</summary> ```dart Text.rich( TextSpan( text: 'This text contains both identifier and label.', semanticsLabel: 'Custom label', semanticsIdentifier: 'Custom identifier', style: customStyle1, children: <TextSpan>[ TextSpan( text: ' While this one contains only label', semanticsLabel: 'Hello world', style: customStyle2, ), const TextSpan( text: ' and this contains only identifier,', semanticsIdentifier: 'Hello to the automation tool', ), TextSpan( text: ' this text contains neither identifier nor label.', style: customStyle2, ), ], ), ), ``` </details> we have the following results with and without the PR code changes: ### With Identifier  ### Without Identifier Changes  ## Semantics Tree Dump The followings are the semantics tree dump for both the cases <details><summary>With Identifier</summary> ``` I/flutter ( 8185): SemanticsNode#0 I/flutter ( 8185): │ Rect.fromLTRB(0.0, 0.0, 1080.0, 2154.0) I/flutter ( 8185): │ I/flutter ( 8185): └─SemanticsNode#1 I/flutter ( 8185): │ Rect.fromLTRB(0.0, 0.0, 392.7, 783.3) scaled by 2.8x I/flutter ( 8185): │ textDirection: ltr I/flutter ( 8185): │ I/flutter ( 8185): └─SemanticsNode#2 I/flutter ( 8185): │ Rect.fromLTRB(0.0, 0.0, 392.7, 783.3) I/flutter ( 8185): │ sortKey: OrdinalSortKey#9e46a(order: 0.0) I/flutter ( 8185): │ I/flutter ( 8185): └─SemanticsNode#3 I/flutter ( 8185): │ Rect.fromLTRB(0.0, 0.0, 392.7, 783.3) I/flutter ( 8185): │ flags: scopesRoute I/flutter ( 8185): │ I/flutter ( 8185): ├─SemanticsNode#4 I/flutter ( 8185): │ Rect.fromLTRB(16.0, 40.0, 376.7, 88.0) I/flutter ( 8185): │ label: "Demonstration of automation tools support in Semantics I/flutter ( 8185): │ for Text and RichText" I/flutter ( 8185): │ textDirection: ltr I/flutter ( 8185): │ I/flutter ( 8185): ├─SemanticsNode#5 I/flutter ( 8185): │ Rect.fromLTRB(16.0, 104.0, 376.7, 204.0) I/flutter ( 8185): │ label: "The identifier property in Semantics widget is used for I/flutter ( 8185): │ UI testing with tools that work by querying the native I/flutter ( 8185): │ accessibility, like UIAutomator, XCUITest, or Appium. It can be I/flutter ( 8185): │ matched with CommonFinders.bySemanticsIdentifier." I/flutter ( 8185): │ textDirection: ltr I/flutter ( 8185): │ I/flutter ( 8185): ├─SemanticsNode#6 I/flutter ( 8185): │ Rect.fromLTRB(16.0, 220.0, 121.9, 244.0) I/flutter ( 8185): │ label: "Text Example:" I/flutter ( 8185): │ textDirection: ltr I/flutter ( 8185): │ I/flutter ( 8185): ├─SemanticsNode#7 I/flutter ( 8185): │ Rect.fromLTRB(16.0, 244.0, 376.7, 304.0) I/flutter ( 8185): │ identifier: "This is a custom identifier that only the automation I/flutter ( 8185): │ tools are able to see" I/flutter ( 8185): │ label: "This is a custom label" I/flutter ( 8185): │ textDirection: ltr I/flutter ( 8185): │ I/flutter ( 8185): ├─SemanticsNode#8 I/flutter ( 8185): │ Rect.fromLTRB(16.0, 320.0, 155.1, 344.0) I/flutter ( 8185): │ label: "Text.rich Example:" I/flutter ( 8185): │ textDirection: ltr I/flutter ( 8185): │ I/flutter ( 8185): ├─SemanticsNode#9 I/flutter ( 8185): │ │ Rect.fromLTRB(16.0, 344.0, 376.7, 400.0) I/flutter ( 8185): │ │ I/flutter ( 8185): │ ├─SemanticsNode#10 I/flutter ( 8185): │ │ Rect.fromLTRB(-4.0, -3.0, 280.0, 23.0) I/flutter ( 8185): │ │ identifier: "Custom identifier" I/flutter ( 8185): │ │ label: "Custom label" I/flutter ( 8185): │ │ textDirection: ltr I/flutter ( 8185): │ │ sortKey: OrdinalSortKey#06bc7(order: 0.0) I/flutter ( 8185): │ │ I/flutter ( 8185): │ ├─SemanticsNode#11 I/flutter ( 8185): │ │ Rect.fromLTRB(-4.0, -1.0, 345.0, 42.0) I/flutter ( 8185): │ │ label: "Hello world" I/flutter ( 8185): │ │ textDirection: ltr I/flutter ( 8185): │ │ sortKey: OrdinalSortKey#32a12(order: 1.0) I/flutter ( 8185): │ │ I/flutter ( 8185): │ ├─SemanticsNode#12 I/flutter ( 8185): │ │ Rect.fromLTRB(130.0, 17.0, 348.0, 43.0) I/flutter ( 8185): │ │ identifier: "Hello to the automation tool" I/flutter ( 8185): │ │ label: " and this contains only identifier," I/flutter ( 8185): │ │ textDirection: ltr I/flutter ( 8185): │ │ sortKey: OrdinalSortKey#49d25(order: 2.0) I/flutter ( 8185): │ │ I/flutter ( 8185): │ └─SemanticsNode#13 I/flutter ( 8185): │ Rect.fromLTRB(-4.0, 19.0, 351.0, 60.0) I/flutter ( 8185): │ label: " this text contains neither identifier nor label." I/flutter ( 8185): │ textDirection: ltr I/flutter ( 8185): │ sortKey: OrdinalSortKey#f3624(order: 3.0) I/flutter ( 8185): │ I/flutter ( 8185): ├─SemanticsNode#14 I/flutter ( 8185): │ Rect.fromLTRB(16.0, 416.0, 181.0, 440.0) I/flutter ( 8185): │ label: "Multi-tenant Example:" I/flutter ( 8185): │ textDirection: ltr I/flutter ( 8185): │ I/flutter ( 8185): ├─SemanticsNode#15 I/flutter ( 8185): │ │ Rect.fromLTRB(108.3, 440.0, 284.5, 480.0) I/flutter ( 8185): │ │ I/flutter ( 8185): │ ├─SemanticsNode#16 I/flutter ( 8185): │ │ Rect.fromLTRB(-1.0, -3.0, 115.0, 23.0) I/flutter ( 8185): │ │ identifier: "please_open" I/flutter ( 8185): │ │ label: "Please open the " I/flutter ( 8185): │ │ textDirection: ltr I/flutter ( 8185): │ │ sortKey: OrdinalSortKey#ea831(order: 0.0) I/flutter ( 8185): │ │ I/flutter ( 8185): │ ├─SemanticsNode#17 I/flutter ( 8185): │ │ Rect.fromLTRB(106.0, -3.0, 177.0, 23.0) I/flutter ( 8185): │ │ identifier: "product_name" I/flutter ( 8185): │ │ label: "product 1" I/flutter ( 8185): │ │ textDirection: ltr I/flutter ( 8185): │ │ sortKey: OrdinalSortKey#589fe(order: 1.0) I/flutter ( 8185): │ │ I/flutter ( 8185): │ ├─SemanticsNode#18 I/flutter ( 8185): │ │ Rect.fromLTRB(-4.0, -3.0, 177.0, 43.0) I/flutter ( 8185): │ │ identifier: "to_use_app" I/flutter ( 8185): │ │ label: I/flutter ( 8185): │ │ " I/flutter ( 8185): │ │ to use this app." I/flutter ( 8185): │ │ textDirection: ltr I/flutter ( 8185): │ │ sortKey: OrdinalSortKey#c2762(order: 2.0) I/flutter ( 8185): │ │ I/flutter ( 8185): │ └─SemanticsNode#19 I/flutter ( 8185): │ Rect.fromLTRB(95.0, 17.0, 181.0, 43.0) I/flutter ( 8185): │ actions: tap I/flutter ( 8185): │ flags: isLink I/flutter ( 8185): │ identifier: "learn_more_link" I/flutter ( 8185): │ label: " Learn more" I/flutter ( 8185): │ textDirection: ltr I/flutter ( 8185): │ sortKey: OrdinalSortKey#7d560(order: 3.0) I/flutter ( 8185): │ I/flutter ( 8185): └─SemanticsNode#20 I/flutter ( 8185): │ Rect.fromLTRB(97.0, 496.0, 295.7, 536.0) I/flutter ( 8185): │ I/flutter ( 8185): ├─SemanticsNode#21 I/flutter ( 8185): │ Rect.fromLTRB(11.0, -3.0, 127.0, 23.0) I/flutter ( 8185): │ identifier: "please_open" I/flutter ( 8185): │ label: "Please open the " I/flutter ( 8185): │ textDirection: ltr I/flutter ( 8185): │ sortKey: OrdinalSortKey#7bb57(order: 0.0) I/flutter ( 8185): │ I/flutter ( 8185): ├─SemanticsNode#22 I/flutter ( 8185): │ Rect.fromLTRB(118.0, -3.0, 188.0, 23.0) I/flutter ( 8185): │ identifier: "product_name" I/flutter ( 8185): │ label: "product 2" I/flutter ( 8185): │ textDirection: ltr I/flutter ( 8185): │ sortKey: OrdinalSortKey#6c7c6(order: 1.0) I/flutter ( 8185): │ I/flutter ( 8185): ├─SemanticsNode#23 I/flutter ( 8185): │ Rect.fromLTRB(-4.0, -3.0, 188.0, 43.0) I/flutter ( 8185): │ identifier: "to_use_app" I/flutter ( 8185): │ label: I/flutter ( 8185): │ " I/flutter ( 8185): │ to access this app." I/flutter ( 8185): │ textDirection: ltr I/flutter ( 8185): │ sortKey: OrdinalSortKey#1e8e7(order: 2.0) I/flutter ( 8185): │ I/flutter ( 8185): └─SemanticsNode#24 I/flutter ( 8185): Rect.fromLTRB(117.0, 17.0, 203.0, 43.0) I/flutter ( 8185): actions: tap I/flutter ( 8185): flags: isLink I/flutter ( 8185): identifier: "learn_more_link" I/flutter ( 8185): label: " Find out more" I/flutter ( 8185): textDirection: ltr I/flutter ( 8185): sortKey: OrdinalSortKey#db7e6(order: 3.0) ``` </details> <details><summary>Without Identifier Changes</summary> ``` I/flutter (18659): SemanticsNode#0 I/flutter (18659): │ Rect.fromLTRB(0.0, 0.0, 1080.0, 2154.0) I/flutter (18659): │ I/flutter (18659): └─SemanticsNode#1 I/flutter (18659): │ Rect.fromLTRB(0.0, 0.0, 392.7, 783.3) scaled by 2.8x I/flutter (18659): │ textDirection: ltr I/flutter (18659): │ I/flutter (18659): └─SemanticsNode#2 I/flutter (18659): │ Rect.fromLTRB(0.0, 0.0, 392.7, 783.3) I/flutter (18659): │ sortKey: OrdinalSortKey#102d4(order: 0.0) I/flutter (18659): │ I/flutter (18659): └─SemanticsNode#3 I/flutter (18659): │ Rect.fromLTRB(0.0, 0.0, 392.7, 783.3) I/flutter (18659): │ flags: scopesRoute I/flutter (18659): │ I/flutter (18659): ├─SemanticsNode#4 I/flutter (18659): │ Rect.fromLTRB(16.0, 40.0, 376.7, 88.0) I/flutter (18659): │ label: "Demonstration of automation tools support in Semantics I/flutter (18659): │ for Text and RichText" I/flutter (18659): │ textDirection: ltr I/flutter (18659): │ I/flutter (18659): ├─SemanticsNode#5 I/flutter (18659): │ Rect.fromLTRB(16.0, 104.0, 376.7, 204.0) I/flutter (18659): │ label: "The identifier property in Semantics widget is used for I/flutter (18659): │ UI testing with tools that work by querying the native I/flutter (18659): │ accessibility, like UIAutomator, XCUITest, or Appium. It can be I/flutter (18659): │ matched with CommonFinders.bySemanticsIdentifier." I/flutter (18659): │ textDirection: ltr I/flutter (18659): │ I/flutter (18659): ├─SemanticsNode#6 I/flutter (18659): │ Rect.fromLTRB(16.0, 220.0, 121.9, 244.0) I/flutter (18659): │ label: "Text Example:" I/flutter (18659): │ textDirection: ltr I/flutter (18659): │ I/flutter (18659): ├─SemanticsNode#7 I/flutter (18659): │ Rect.fromLTRB(16.0, 244.0, 376.7, 304.0) I/flutter (18659): │ label: "This is a custom label" I/flutter (18659): │ textDirection: ltr I/flutter (18659): │ I/flutter (18659): ├─SemanticsNode#8 I/flutter (18659): │ Rect.fromLTRB(16.0, 320.0, 155.1, 344.0) I/flutter (18659): │ label: "Text.rich Example:" I/flutter (18659): │ textDirection: ltr I/flutter (18659): │ I/flutter (18659): ├─SemanticsNode#9 I/flutter (18659): │ Rect.fromLTRB(16.0, 344.0, 376.7, 400.0) I/flutter (18659): │ label: "Custom labelHello world and this contains only I/flutter (18659): │ identifier, this text contains neither identifier nor label." I/flutter (18659): │ textDirection: ltr I/flutter (18659): │ I/flutter (18659): ├─SemanticsNode#10 I/flutter (18659): │ Rect.fromLTRB(16.0, 416.0, 181.0, 440.0) I/flutter (18659): │ label: "Multi-tenant Example:" I/flutter (18659): │ textDirection: ltr I/flutter (18659): │ I/flutter (18659): ├─SemanticsNode#11 I/flutter (18659): │ │ Rect.fromLTRB(108.3, 456.0, 284.5, 496.0) I/flutter (18659): │ │ I/flutter (18659): │ ├─SemanticsNode#12 I/flutter (18659): │ │ Rect.fromLTRB(-4.0, -3.0, 177.0, 43.0) I/flutter (18659): │ │ label: I/flutter (18659): │ │ "Please open the product 1 I/flutter (18659): │ │ to use this app." I/flutter (18659): │ │ textDirection: ltr I/flutter (18659): │ │ sortKey: OrdinalSortKey#493fc(order: 0.0) I/flutter (18659): │ │ I/flutter (18659): │ └─SemanticsNode#13 I/flutter (18659): │ Rect.fromLTRB(95.0, 17.0, 181.0, 43.0) I/flutter (18659): │ actions: tap I/flutter (18659): │ flags: isLink I/flutter (18659): │ label: " Learn more" I/flutter (18659): │ textDirection: ltr I/flutter (18659): │ sortKey: OrdinalSortKey#587bf(order: 1.0) I/flutter (18659): │ I/flutter (18659): └─SemanticsNode#14 I/flutter (18659): │ Rect.fromLTRB(88.9, 512.0, 303.8, 552.0) I/flutter (18659): │ I/flutter (18659): ├─SemanticsNode#15 I/flutter (18659): │ Rect.fromLTRB(-4.0, -3.0, 196.0, 43.0) I/flutter (18659): │ label: I/flutter (18659): │ "Please open the product 2 I/flutter (18659): │ to access this app." I/flutter (18659): │ textDirection: ltr I/flutter (18659): │ sortKey: OrdinalSortKey#69083(order: 0.0) I/flutter (18659): │ I/flutter (18659): └─SemanticsNode#16 I/flutter (18659): Rect.fromLTRB(117.0, 17.0, 219.0, 43.0) I/flutter (18659): actions: tap I/flutter (18659): flags: isLink I/flutter (18659): label: " Find out more" I/flutter (18659): textDirection: ltr I/flutter (18659): sortKey: OrdinalSortKey#ed706(order: 1.0) ``` </details> fixes flutter#163842 --------- Co-authored-by: chunhtai <[email protected]>
This PR aims to add
semanticsIdentifiertoTextand some of its internal objects to pass the semantics information for adding identifier to the semantics nodesFrom the issue filed at #163842, the following is a description of the problem.
The semantics identifier helps in uniquely identifying elements using UI automation tools like Appium, UIAutomator and XCUITests by setting identifiers that the screen readers cannot see but the said tools can. This is especially useful when working with a multi-lingual or multi-tenant app, where the element IDs need to be unique but the content can be different. The
Semanticswidget already has support for declaring it. However, theTextandText.richvariants only support settingsemanticsLabelwithout explicitly setting the identifiers. The widgets themselves can be wrapped with aSemanticswidget but it still does not cater for a rich text that can have multiple text spans, each containing unique lables and identifiers, and optionally gesture detectors for handling links.Consider the following UI for two different tenants:

Here, both the tenants utilise different strings to convey the same message. The structure of the message stays the same so the identifiers help in unifying the element identification process across the tenant apps in the automation tools without having to write another script for every other tenant.
Without the identifiers, the automation scripts require a rewrite per tenant to be able to successfully locate the element and even tap on the hyperlink.
With PR Changes
Appium Views
For the given sample code,
Text.rich Sample
With Identifier
Without Identifier Changes
Semantics Tree Dump
The followings are the semantics tree dump for both the cases
With Identifier
Without Identifier Changes
fixes #163842