[Android R] Sync keyboard animation with view insets vs Android 11/R/API 30 WindowInsetsAnimation #20843

GaryQian · 2020-08-28T03:44:29Z

Adds a WindowInsetsAnimation.Callback and OnApplyWindowInsetsListener class that handles sending view inset changes from the keyboard animating to the framework.

When keyboard is shown or hidden, a call to onApplyWindowInsets() is made immediately with the keyboard's final state. This causes a flicker as the animation kicks in a frame later. To solve this, we check for this initial call and capture the state, deferring it being called until the animation is completed.

During the animation, we pass the window insets to the framework on each frame.

There is an issue where the onProgress WindowInsets received has a bottom ime inset that is higher than expected. It is actually the sum of the navigation bars on top of the keyboard height. There is a workaround implemented to merge the navbar height with the keyboard height to resolve this.

Partially resolves flutter/flutter#62876

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

xster · 2020-09-03T23:10:43Z

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

+      if ((View.SYSTEM_UI_FLAG_HIDE_NAVIGATION & mView.getWindowSystemUiVisibility()) == 0) {
+        mask = mask | WindowInsets.Type.navigationBars();
+      }
+      if ((View.SYSTEM_UI_FLAG_FULLSCREEN & mView.getWindowSystemUiVisibility()) == 0) {


does full screen also independently affect navigation bar above too?

These are the two values we use to compute viewPadding, so for consistency, I include it together to keep in sync with FlutterView.onApplyWindowInsets. Any padding we consume will also be accounted for here this way.

xster · 2020-09-03T23:11:58Z

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

+              mask, // Overlay
+              WindowInsets.Type.ime() // Deferred
+              );
+      view.setWindowInsetsAnimationCallback(imeSyncCallback);


add comment on why you need 2

xster · 2020-09-03T23:12:28Z

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

+              WindowInsets.Type.ime() // Deferred
+              );
+      view.setWindowInsetsAnimationCallback(imeSyncCallback);
+      view.setOnApplyWindowInsetsListener(imeSyncCallback);


do you remove this in destroy?

xster · 2020-09-03T23:13:54Z

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

+  // Loosely based off of
+  // https://github.com/android/user-interface-samples/blob/master/WindowInsetsAnimation/app/src/main/java/com/google/android/samples/insetsanimation/RootViewDeferringInsetsCallback.kt
+  // When the IME is shown or hidden, it sends an onApplyWindowInsets call with the
+  // final state of the IME. This defers the final call to allow the animation to


Sentence not clear to me. What's 'this'? 'final call' to what?

xster · 2020-09-03T23:17:55Z

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

+      super(WindowInsetsAnimation.Callback.DISPATCH_MODE_CONTINUE_ON_SUBTREE);
+      this.overlayInsetTypes = overlayInsetTypes;
+      this.deferredInsetTypes = deferredInsetTypes;
+      this.view = view;


preconditions checknotnull

xster · 2020-09-03T23:21:00Z

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

+    @Override
+    public WindowInsets onApplyWindowInsets(View view, WindowInsets windowInsets) {
+      this.view = view;
+      if (!started) {


ya, these apis are pretty confusing. I think this needs a lot more comments. Comments at the class level for what the flow should be for the persistent vs delayed stuff are. When this !started should happen, when should deferredInsets happen.

i.e. I don't think the next maintainer would know what to do here if there was a bug and you're out.

I'll make a pass and document the whole thing

xster · 2020-09-03T23:24:18Z

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

+    @Override
+    public WindowInsetsAnimation.Bounds onStart(
+        WindowInsetsAnimation animation, WindowInsetsAnimation.Bounds bounds) {
+      if (deferredInsets && (animation.getTypeMask() & deferredInsetTypes) != 0) {


comments here too. What's the difference between onprepare and onstart? i.e. why not just set both deferredInsets and started to true here? Can you start and not prepare?

I initially thought that both were necessary, turns out only start is required.

xster · 2020-09-03T23:25:21Z

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

+                      - insets.getInsets(overlayInsetTypes).bottom,
+                  0));
+      builder.setInsets(deferredInsetTypes, newImeInsets);
+      // Directly call onApplyWindowInsets as we want to skip this class' version of this call.


This isn't super clear. Maybe elaborate a bit more.

xster · 2020-09-03T23:26:11Z

shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java

+  @Config(sdk = 30)
+  public void ime_windowInsetsSync() {
+    if (Build.VERSION.SDK_INT < Build.VERSION_CODES.R) {
+      return;


if you say this needs sdk 30 and it's not sdk 30, we should just crash :D

xster

LGTM

xster · 2020-09-04T07:26:08Z

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

+  // Loosely based off of
+  // https://github.com/android/user-interface-samples/blob/master/WindowInsetsAnimation/app/src/main/java/com/google/android/samples/insetsanimation/RootViewDeferringInsetsCallback.kt
+  //
+  // When the IME is shown or hidden, it immediately sends an onApplyWindowInsets call


Ah, this is much better, thanks! Does this class also handle all the non-animating inset changes? Like the nav bar, status bar going in and out of fullscreen etc? How do all the phases like onStart, onApplyWindowInsets etc affect those insets?

Non-animating changes are just passed right on through to FlutterView.onApplyWindowInsets without impact. The ime-type filtering also prevents this from doing anything on any non-ime animations. During animations, changes to the window are handled by the animation controller, which will end the current animation, and restart a new one with the modified settings to continue. For example, rotating the device mid-keyboard animation just causes the animation to continue where it left off but scaled to the rotated window.

xster · 2020-09-04T07:31:44Z

shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java

+    ArgumentCaptor<FlutterRenderer.ViewportMetrics> viewportMetricsCaptor =
+        ArgumentCaptor.forClass(FlutterRenderer.ViewportMetrics.class);
+
+    imeSyncCallback.onApplyWindowInsets(testView, deferredInsets);


I was gonna say I'm surprised you have to do this on your callback directly but then I actually can't find this on any of the robolectric shadows https://github.com/robolectric/robolectric/search?q=onApplyWindowInsets 🤷‍♂️

🤷‍♂️

xster · 2020-09-04T23:47:24Z

🙌

…id 11/R/API 30 WindowInsetsAnimation (flutter/engine#20843)

* 30b829e Populates fuchsia node actions in semantics updates. (flutter/engine#20451) * c2e7010 Roll Skia from 1ee21cdfb6fe to 6763a713f957 (1 revision) (flutter/engine#20982) * 841401d restore FML_DCHECK removed due to a code reviewing error (flutter/engine#20953) * 367c6db Don't use GetTaskQueueId() in rasterizer as it breaks Fuchsia (flutter/engine#20983) * b22a8c6 Let FlutterActivity/Fragment/FragmentActivity have an app bundle path override instead of eager resolving during construction (flutter/engine#20769) * 0f0ae68 Update test Dart code to pass the latest Dart analyzer rules (flutter/engine#20986) * d77dd31 Manual roll of Dart b29f228f62...016e8880f0 (flutter/engine#20967) * c7b3d53 Roll Fuchsia Mac SDK from gOI3W1UNU... to EN2ycWLxi... (flutter/engine#20985) * 6a6986d improve sensitivity of BackdropFilter web tests (flutter/engine#20915) * 9fc9cb2 Roll Dart SDK from 016e8880f0ab to 0f0cff3922ad (7 revisions) (flutter/engine#20990) * 242d522 [Android R] Sync keyboard animation with view insets vs Android 11/R/API 30 WindowInsetsAnimation (flutter/engine#20843) * b4e0896 Roll Fuchsia Linux SDK from 81O8Kg_Rw... to A0PKwETay... (flutter/engine#20998) * d77c4e5 adjust blur radius for HTML to match CanvasKit (flutter/engine#20840) * 0628492 Roll Dart SDK from 0f0cff3922ad to f3a9ca88b664 (1 revision) (flutter/engine#21000) * d1d848e Roll Fuchsia Mac SDK from EN2ycWLxi... to sih5f60Gt... (flutter/engine#20999)

GaryQian added affects: engine platform-android affects: text input Work in progress (WIP) Not ready (yet) for review! labels Aug 28, 2020

GaryQian self-assigned this Aug 28, 2020

googlebot added the cla: yes label Aug 28, 2020

GaryQian force-pushed the windowinsets branch 2 times, most recently from 155c5d5 to a6bef30 Compare September 3, 2020 12:51

GaryQian changed the title ~~[WIP] setKeyboardInset Text Channel to manually set keyboard inset position~~ [WIP] Sync keyboard animation with view insets vs Android 11/R/API 30 WindowInsetsAnimation Sep 3, 2020

GaryQian marked this pull request as ready for review September 3, 2020 21:13

GaryQian requested review from jason-simmons and xster September 3, 2020 21:13

GaryQian changed the title ~~[WIP] Sync keyboard animation with view insets vs Android 11/R/API 30 WindowInsetsAnimation~~ [Android R] Sync keyboard animation with view insets vs Android 11/R/API 30 WindowInsetsAnimation Sep 3, 2020

GaryQian removed the Work in progress (WIP) Not ready (yet) for review! label Sep 3, 2020

GaryQian mentioned this pull request Sep 3, 2020

Support synchronized IME on Android flutter/flutter#62876

Closed

xster reviewed Sep 3, 2020

View reviewed changes

GaryQian added 11 commits September 3, 2020 16:54

Initial code

dfaa942

Additional core impl

af4d7a8

PArtially working

7f03868

DispatchOnAttach

794304f

Embedder->framework sync initial

3642f61

Working

bc5c37f

Cleanup

cc6399c

Remove framework->embedder

45dbba4

More cleanup

693fe22

Version gate, add test

158f3e4

Formatting

0d8a5e6

GaryQian force-pushed the windowinsets branch 2 times, most recently from 4613d49 to 25f9d52 Compare September 4, 2020 01:18

Docs, bugfixes, linter

e7b3adc

GaryQian force-pushed the windowinsets branch from 67e924d to e7b3adc Compare September 4, 2020 01:40

Remove extra version check

6b0bfae

GaryQian requested a review from xster September 4, 2020 01:55

xster reviewed Sep 4, 2020

View reviewed changes

GaryQian merged commit 242d522 into flutter:master Sep 4, 2020

engine-flutter-autoroll mentioned this pull request Sep 5, 2020

Roll Engine from ac8b9c4c52ea to d1d848e8421e (15 revisions) flutter/flutter#65275

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 5, 2020

242d522 [Android R] Sync keyboard animation with view insets vs Andro…

5648f9e

…id 11/R/API 30 WindowInsetsAnimation (flutter/engine#20843)

GaryQian mentioned this pull request Sep 8, 2020

Soft keyboard animation unsynchronized with Flutter resize animation flutter/flutter#19279

Closed

GaryQian mentioned this pull request Oct 19, 2020

Android 11 Issues and Features flutter/flutter#51365

Closed

Piinks mentioned this pull request Oct 21, 2021

SystemUiMode.edgeToEdge bottom viewInset jumps when keyboard is opening flutter/flutter#89914

Closed

[Android R] Sync keyboard animation with view insets vs Android 11/R/API 30 WindowInsetsAnimation #20843

[Android R] Sync keyboard animation with view insets vs Android 11/R/API 30 WindowInsetsAnimation #20843

Uh oh!

Conversation

GaryQian commented Aug 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xster left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xster commented Sep 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GaryQian commented Aug 28, 2020 •

edited

Loading