This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Android Embedding PR 17: Clarify AccessibilityBridge. #8061
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
matthew-carroll
requested review from
cbracken,
goderbauer,
dnfield and
jonahwilliams
shell/platform/android/io/flutter/embedding/engine/systemchannels/AccessibilityChannel.java
Show resolved
Hide resolved
shell/platform/android/io/flutter/view/AccessibilityBridge.java
Outdated
Show resolved
Hide resolved
shell/platform/android/io/flutter/view/AccessibilityBridge.java
Outdated
Show resolved
Hide resolved
shell/platform/android/io/flutter/view/AccessibilityBridge.java
Outdated
Show resolved
Hide resolved
shell/platform/android/io/flutter/view/AccessibilityBridge.java
Outdated
Show resolved
Hide resolved
shell/platform/android/io/flutter/view/AccessibilityBridge.java
Outdated
Show resolved
Hide resolved
shell/platform/android/io/flutter/view/AccessibilityBridge.java
Outdated
Show resolved
Hide resolved
dnfield
reviewed
Mar 6, 2019
|
This LGTM, although I'd defer to Jonah for full review |
|
@jonahwilliams I just committed round 2 of comments. There are many more doc comments, some renames for clarity, and some new TODOs with questions. I haven't gotten to all of those |
shell/platform/android/io/flutter/view/AccessibilityBridge.java
Outdated
Show resolved
Hide resolved
shell/platform/android/io/flutter/view/AccessibilityBridge.java
Outdated
Show resolved
Hide resolved
shell/platform/android/io/flutter/view/AccessibilityBridge.java
Outdated
Show resolved
Hide resolved
shell/platform/android/io/flutter/view/AccessibilityBridge.java
Outdated
Show resolved
Hide resolved
shell/platform/android/io/flutter/view/AccessibilityBridge.java
Outdated
Show resolved
Hide resolved
shell/platform/android/io/flutter/view/AccessibilityBridge.java
Outdated
Show resolved
Hide resolved
…ibility stuff from FlutterView to AccessibilityBridge.
|
@jonahwilliams I just pushed round 3. This one moves a lot of stuff from Also adding @mklim to this PR because it probably overlaps with some of his API guarding work. |
…cessibility_bridge
The new error is really an existing error, just moved to a new file. Not fixing it immediately because this lint is actually arguable. We may want to turn it off entirely in the future.
… also corrected some comment typos.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Mar 9, 2019
… logic out of FlutterView. (flutter/engine#8061)
engine-flutter-autoroll
added a commit
to flutter/flutter
that referenced
this pull request
Mar 9, 2019
flutter/engine@c48774c...03d5f3c git log c48774c..03d5f3c --no-merges --oneline 03d5f3c Clarify arguments to FlutterEngineOnVsync (flutter/engine#8093) 718329c Android Embedding PR 17: Clarify AccessibilityBridge and move logic out of FlutterView. (flutter/engine#8061) edfc0cf Android Embedding PR 16: Add touch support to FlutterView. (flutter/engine#8034) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff (mklim@google.com), and stop the roller if necessary.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is strictly aimed at clarifying
AccessibilityBridge(along with a few minor refactors to help pave the way to the new embedding).Please read through all new comments to help validate them. Also, if you know the answer to any of the TODO questions, please comment with the answer and I'll update the code.
UPDATE: I ended up moving quite a bit of logic from
FlutterViewtoAccessibilityBridgebecause the division of responsibility was rather messy and unclear. This refactor also means less duplication between the oldFlutterViewand the new one. Additionally, the lifespan of various accessibility objects and listeners should now be more clear.