Adds semantics tooltip support by chunhtai · Pull Request #27893 · flutter/engine

chunhtai · 2021-08-04T20:13:32Z

framework part: flutter/flutter#87684

issue flutter/flutter#86577

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 and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

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

chunhtai · 2021-08-09T20:39:37Z

cc @yjbanov for web
cc @abrush21 for fuchsia change

lib/ui/semantics.dart

shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java

shell/platform/android/io/flutter/view/AccessibilityBridge.java

blasten · 2021-08-13T01:10:15Z

shell/platform/android/io/flutter/view/AccessibilityBridge.java

+      if (Build.VERSION.SDK_INT < Build.VERSION_CODES.P) {
+        if (semanticsNode.tooltip != null) {
+          content = content != null ? content : "";
+          content = content + "\n" + semanticsNode.tooltip;


comment about why a new line and the intended behavior would be appreciated.

Is it possible to test this logic?

blasten · 2021-08-13T01:13:20Z

shell/platform/android/io/flutter/view/AccessibilityBridge.java

      }
    }

+    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {


what is the intended behavior below this version? The code in L832 required !semanticsNode.hasFlag(Flag.SCOPES_ROUTE), is it always true in API level < Build.VERSION_CODES.P.

If not, why?

SCOPES_ROUTE doesn't need content description because it is not a11y focusable. This is already the case before the change.

and unfortunately, I don't know how to set up test to be run in older API level. It complains about missing binary if i set the config in Robolectric test.

could we add comments? Sorry, I think this will help the next person trying to contribute :)

Imagine that someone jumps into this code for this first time, and that person is trying to make sense of each case, etc...

shell/platform/android/io/flutter/view/AccessibilityBridge.java

chunhtai · 2021-08-13T22:00:55Z

The build is failing because it needs a three way transition to roll into flutter, will do that once both engine and framework change is approved

@blasten I either replied to the comment or resolved them, PTAL

blasten · 2021-08-14T00:06:13Z

shell/platform/android/io/flutter/view/AccessibilityBridge.java

      CharSequence content = semanticsNode.getValueLabelHint();
+      if (Build.VERSION.SDK_INT < Build.VERSION_CODES.P) {
+        if (semanticsNode.tooltip != null) {
+          // For backward compatibility, the tooltip is appended


backward compatibility with which API level, or with Flutter SDK?

chunhtai · 2021-08-16T22:58:16Z

@blasten PTAL :)

blasten

LGTM

fluttergithubbot · 2021-08-18T21:42:02Z

This pull request is not suitable for automatic merging in its current state.

The status or check suite Linux Web Framework tests has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Windows Host Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

This reverts commit 14e27f9.

flutter-dashboard bot added embedder Related to the embedder API platform-android platform-fuchsia labels Aug 4, 2021

google-cla bot added the cla: yes label Aug 4, 2021

chunhtai mentioned this pull request Aug 4, 2021

Adds tooltip to semantics node flutter/flutter#87684

Merged

8 tasks

chunhtai force-pushed the issues/86577 branch 2 times, most recently from 9b66963 to d4155eb Compare August 9, 2021 20:30

chunhtai requested review from blasten and goderbauer August 9, 2021 20:39

chunhtai force-pushed the issues/86577 branch from d4155eb to d88ea36 Compare August 9, 2021 20:50

blasten reviewed Aug 10, 2021

View reviewed changes

lib/ui/semantics.dart Show resolved Hide resolved

blasten reviewed Aug 13, 2021

View reviewed changes

shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java Show resolved Hide resolved

blasten reviewed Aug 13, 2021

View reviewed changes

shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java Show resolved Hide resolved

blasten reviewed Aug 13, 2021

View reviewed changes

shell/platform/android/io/flutter/view/AccessibilityBridge.java Outdated Show resolved Hide resolved

blasten reviewed Aug 13, 2021

View reviewed changes

shell/platform/android/io/flutter/view/AccessibilityBridge.java Outdated Show resolved Hide resolved

chunhtai force-pushed the issues/86577 branch from 60ddf7a to 243fef7 Compare August 13, 2021 20:51

blasten reviewed Aug 14, 2021

View reviewed changes

chunhtai requested a review from blasten August 16, 2021 22:58

blasten approved these changes Aug 16, 2021

View reviewed changes

chunhtai mentioned this pull request Aug 16, 2021

Enable soft transition for tooltip flutter/flutter#88318

Merged

8 tasks

chunhtai added 3 commits August 18, 2021 13:37

Adds semantics tooltip support

6b85679

Addressing comments

b94f9bf

addressing comment

c57af8a

chunhtai force-pushed the issues/86577 branch from 9179f48 to c57af8a Compare August 18, 2021 20:37

chunhtai added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Aug 18, 2021

fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Aug 18, 2021

chunhtai added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Aug 18, 2021

fluttergithubbot merged commit 14e27f9 into flutter:master Aug 18, 2021

engine-flutter-autoroll mentioned this pull request Aug 18, 2021

Roll Engine from 4c7e33a4248e to bf63412ef75d (2 revisions) flutter/flutter#88481

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 18, 2021

14e27f9 Adds semantics tooltip support (flutter/engine#27893)

1e0fd85

zanderso mentioned this pull request Aug 19, 2021

Flaky Mac module_test_ios now failing regularly flutter/flutter#88495

Closed

jmagman mentioned this pull request Aug 19, 2021

iOS apps crashing, -[SemanticsObject accessibilityAttributedLabel], objc_msgSend() selector name: length flutter/flutter#88547

Closed

zanderso added a commit that referenced this pull request Aug 19, 2021

Revert "Adds semantics tooltip support (#27893)"

27aecbd

This reverts commit 14e27f9.

zanderso mentioned this pull request Aug 19, 2021

Revert "Adds semantics tooltip support" #28207

Merged

zanderso added a commit that referenced this pull request Aug 19, 2021

Revert "Adds semantics tooltip support (#27893)" (#28207)

b20c1a3

This reverts commit 14e27f9.

chriscraws pushed a commit to chriscraws/engine that referenced this pull request Aug 23, 2021

Adds semantics tooltip support (flutter#27893)

a9146ca

chriscraws pushed a commit to chriscraws/engine that referenced this pull request Aug 23, 2021

Revert "Adds semantics tooltip support (flutter#27893)" (flutter#28207)

77c8bbe

This reverts commit 14e27f9.

filmil pushed a commit to filmil/engine that referenced this pull request Apr 21, 2022

Adds semantics tooltip support (flutter#27893)

b26f7c7

filmil pushed a commit to filmil/engine that referenced this pull request Apr 21, 2022

Revert "Adds semantics tooltip support (flutter#27893)" (flutter#28207)

7bebf2e

This reverts commit 14e27f9.

Conversation

chunhtai commented Aug 4, 2021

Pre-launch Checklist

Uh oh!

chunhtai commented Aug 9, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

blasten Aug 13, 2021

Choose a reason for hiding this comment

Uh oh!

blasten Aug 13, 2021

Choose a reason for hiding this comment

Uh oh!

chunhtai Aug 13, 2021

Choose a reason for hiding this comment

Uh oh!

chunhtai Aug 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blasten Aug 14, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chunhtai commented Aug 13, 2021

Uh oh!

blasten Aug 14, 2021

Choose a reason for hiding this comment

Uh oh!

chunhtai commented Aug 16, 2021

Uh oh!

blasten left a comment

Choose a reason for hiding this comment

Uh oh!

fluttergithubbot commented Aug 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chunhtai Aug 13, 2021 •

edited

Loading