From 6945789a8b80779385341da0ac95d647e53aaf25 Mon Sep 17 00:00:00 2001 From: Jonathan Cole Date: Sat, 4 Feb 2023 17:34:32 +0000 Subject: [PATCH 1/4] Fix navigation bar insets incorrectly being subtracted from keyboard insets in some cases --- .../ImeSyncDeferringInsetsCallback.java | 37 ++++++++----------- .../plugin/editing/TextInputPlugin.java | 15 +------- 2 files changed, 17 insertions(+), 35 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/editing/ImeSyncDeferringInsetsCallback.java b/shell/platform/android/io/flutter/plugin/editing/ImeSyncDeferringInsetsCallback.java index 0a6f0ef3f64db..6c011ac2ccc99 100644 --- a/shell/platform/android/io/flutter/plugin/editing/ImeSyncDeferringInsetsCallback.java +++ b/shell/platform/android/io/flutter/plugin/editing/ImeSyncDeferringInsetsCallback.java @@ -47,9 +47,7 @@ @SuppressLint({"NewApi", "Override"}) @Keep class ImeSyncDeferringInsetsCallback { - private int overlayInsetTypes; - private int deferredInsetTypes; - + private final int deferredInsetTypes = WindowInsets.Type.ime(); private View view; private WindowInsets lastWindowInsets; private AnimationCallback animationCallback; @@ -67,10 +65,7 @@ class ImeSyncDeferringInsetsCallback { // initial WindowInset. private boolean needsSave = false; - ImeSyncDeferringInsetsCallback( - @NonNull View view, int overlayInsetTypes, int deferredInsetTypes) { - this.overlayInsetTypes = overlayInsetTypes; - this.deferredInsetTypes = deferredInsetTypes; + ImeSyncDeferringInsetsCallback(@NonNull View view) { this.view = view; this.animationCallback = new AnimationCallback(); this.insetsListener = new InsetsListener(); @@ -131,24 +126,24 @@ public WindowInsets onProgress( if (!matching) { return insets; } + + // The IME insets include the height of the navigation bar. If the app isn't laid out behind + // the navigation bar, this causes the IME insets will be too large during the animation. + // To fix this, we subtract the systemBars bottom inset if the system UI flags for laying out + // behind the navigation bar aren't present. + int excludedInsets = 0; + int systemUiFlags = view.getWindowSystemUiVisibility(); + if ((systemUiFlags & View.SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION) == 0 + && (systemUiFlags & View.SYSTEM_UI_FLAG_HIDE_NAVIGATION) == 0) { + excludedInsets = insets.getInsets(WindowInsets.Type.systemBars()).bottom; + } + WindowInsets.Builder builder = new WindowInsets.Builder(lastWindowInsets); - // Overlay the ime-only insets with the full insets. - // - // The IME insets passed in by onProgress assumes that the entire animation - // occurs above any present navigation and status bars. This causes the - // IME inset to be too large for the animation. To remedy this, we merge the - // IME inset with other insets present via a subtract + reLu, which causes the - // IME inset to be overlaid with any bars present. Insets newImeInsets = Insets.of( - 0, - 0, - 0, - Math.max( - insets.getInsets(deferredInsetTypes).bottom - - insets.getInsets(overlayInsetTypes).bottom, - 0)); + 0, 0, 0, Math.max(insets.getInsets(deferredInsetTypes).bottom - excludedInsets, 0)); builder.setInsets(deferredInsetTypes, newImeInsets); + // Directly call onApplyWindowInsets of the view as we do not want to pass through // the onApplyWindowInsets defined in this class, which would consume the insets // as if they were a non-animation inset change and cache it for re-dispatch in diff --git a/shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java b/shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java index 10cbc1b65cb61..f38a48f53a0d3 100644 --- a/shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java +++ b/shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java @@ -15,7 +15,6 @@ import android.view.KeyEvent; import android.view.View; import android.view.ViewStructure; -import android.view.WindowInsets; import android.view.autofill.AutofillId; import android.view.autofill.AutofillManager; import android.view.autofill.AutofillValue; @@ -80,19 +79,7 @@ public TextInputPlugin( // the Flutter view to grow and shrink to accommodate Android // controlled keyboard animations. if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { - int mask = 0; - if ((View.SYSTEM_UI_FLAG_HIDE_NAVIGATION & mView.getWindowSystemUiVisibility()) == 0) { - mask = mask | WindowInsets.Type.navigationBars(); - } - if ((View.SYSTEM_UI_FLAG_FULLSCREEN & mView.getWindowSystemUiVisibility()) == 0) { - mask = mask | WindowInsets.Type.statusBars(); - } - imeSyncCallback = - new ImeSyncDeferringInsetsCallback( - view, - mask, // Overlay, insets that should be merged with the deferred insets - WindowInsets.Type.ime() // Deferred, insets that will animate - ); + imeSyncCallback = new ImeSyncDeferringInsetsCallback(view); imeSyncCallback.install(); } From e63b10f23f7981585022876da2adb6674dd53996 Mon Sep 17 00:00:00 2001 From: Jonathan Cole Date: Sat, 4 Feb 2023 19:23:45 +0000 Subject: [PATCH 2/4] Update tests --- .../plugin/editing/TextInputPluginTest.java | 138 +++++++++++++++--- 1 file changed, 118 insertions(+), 20 deletions(-) diff --git a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java index 462b6ec2ff808..ee23124366072 100644 --- a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java +++ b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java @@ -2029,8 +2029,10 @@ public void sendAppPrivateCommand_hasData() throws JSONException { @Test @TargetApi(30) @Config(sdk = 30) - public void ime_windowInsetsSync() { - FlutterView testView = new FlutterView(Robolectric.setupActivity(Activity.class)); + public void ime_windowInsetsSync_notLaidOutBehindNavigation_excludesSystemBars() { + FlutterView testView = spy(new FlutterView(Robolectric.setupActivity(Activity.class))); + when(testView.getWindowSystemUiVisibility()).thenReturn(View.SYSTEM_UI_FLAG_LAYOUT_STABLE); + TextInputChannel textInputChannel = new TextInputChannel(mock(DartExecutor.class)); TextInputPlugin textInputPlugin = new TextInputPlugin(testView, textInputChannel, mock(PlatformViewsController.class)); @@ -2049,28 +2051,122 @@ public void ime_windowInsetsSync() { WindowInsets.Builder builder = new WindowInsets.Builder(); WindowInsets noneInsets = builder.build(); - // imeInsets0, 1, and 2 contain unique IME bottom insets, and are used - // to distinguish which insets were sent at each stage. - builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 100)); - builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(10, 10, 10, 40)); + builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 25)); + builder.setInsets(WindowInsets.Type.systemBars(), Insets.of(10, 10, 10, 40)); WindowInsets imeInsets0 = builder.build(); - builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 30)); - builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(10, 10, 10, 40)); + builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 50)); + builder.setInsets(WindowInsets.Type.systemBars(), Insets.of(10, 10, 10, 40)); WindowInsets imeInsets1 = builder.build(); + builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 100)); + builder.setInsets(WindowInsets.Type.systemBars(), Insets.of(10, 10, 10, 0)); + WindowInsets deferredInsets = builder.build(); + + builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 50)); + builder.setInsets(WindowInsets.Type.systemBars(), Insets.of(10, 10, 10, 40)); + WindowInsets altDeferredInsets = builder.build(); + + ArgumentCaptor viewportMetricsCaptor = + ArgumentCaptor.forClass(FlutterRenderer.ViewportMetrics.class); + + // Set the initial insets + imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, noneInsets); + + verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); + assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); + assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); + assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom); + assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); + + // Set the deferred insets and make a second call after the animation start which should be + // ignored + imeSyncCallback.getAnimationCallback().onPrepare(animation); + imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, deferredInsets); + imeSyncCallback.getAnimationCallback().onStart(animation, null); + imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, altDeferredInsets); + + verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); + assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); + assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); + assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom); + assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); + + // Apply the inset animation values + imeSyncCallback.getAnimationCallback().onProgress(imeInsets0, animationList); + + verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); + assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); + assertEquals(10, viewportMetricsCaptor.getValue().viewPaddingTop); + assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom); + assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); + + imeSyncCallback.getAnimationCallback().onProgress(imeInsets1, animationList); + + verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); + assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); + assertEquals(10, viewportMetricsCaptor.getValue().viewPaddingTop); + assertEquals(10, viewportMetricsCaptor.getValue().viewInsetBottom); + assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); + + // At the end of the animation, the insets should match deferredInsets as the call to set + // altDeferredInsets should have been ignored + imeSyncCallback.getAnimationCallback().onEnd(animation); + + verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); + assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); + assertEquals(10, viewportMetricsCaptor.getValue().viewPaddingTop); + assertEquals(100, viewportMetricsCaptor.getValue().viewInsetBottom); + assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); + } + + @Test + @TargetApi(30) + @Config(sdk = 30) + public void ime_windowInsetsSync_laidOutBehindNavigation_includesSystemBars() { + FlutterView testView = spy(new FlutterView(Robolectric.setupActivity(Activity.class))); + when(testView.getWindowSystemUiVisibility()) + .thenReturn( + View.SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION | View.SYSTEM_UI_FLAG_HIDE_NAVIGATION); + + TextInputChannel textInputChannel = new TextInputChannel(mock(DartExecutor.class)); + TextInputPlugin textInputPlugin = + new TextInputPlugin(testView, textInputChannel, mock(PlatformViewsController.class)); + ImeSyncDeferringInsetsCallback imeSyncCallback = textInputPlugin.getImeSyncCallback(); + FlutterEngine flutterEngine = spy(new FlutterEngine(ctx, mockFlutterLoader, mockFlutterJni)); + FlutterRenderer flutterRenderer = spy(new FlutterRenderer(mockFlutterJni)); + when(flutterEngine.getRenderer()).thenReturn(flutterRenderer); + testView.attachToFlutterEngine(flutterEngine); + + WindowInsetsAnimation animation = mock(WindowInsetsAnimation.class); + when(animation.getTypeMask()).thenReturn(WindowInsets.Type.ime()); + + List animationList = new ArrayList(); + animationList.add(animation); + + WindowInsets.Builder builder = new WindowInsets.Builder(); + WindowInsets noneInsets = builder.build(); + + builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 25)); + builder.setInsets(WindowInsets.Type.systemBars(), Insets.of(10, 10, 10, 40)); + WindowInsets imeInsets0 = builder.build(); + builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 50)); - builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(10, 10, 10, 40)); - WindowInsets imeInsets2 = builder.build(); + builder.setInsets(WindowInsets.Type.systemBars(), Insets.of(10, 10, 10, 40)); + WindowInsets imeInsets1 = builder.build(); - builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 200)); - builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(10, 10, 10, 0)); + builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 100)); + builder.setInsets(WindowInsets.Type.systemBars(), Insets.of(10, 10, 10, 0)); WindowInsets deferredInsets = builder.build(); + builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 50)); + builder.setInsets(WindowInsets.Type.systemBars(), Insets.of(10, 10, 10, 40)); + WindowInsets altDeferredInsets = builder.build(); + ArgumentCaptor viewportMetricsCaptor = ArgumentCaptor.forClass(FlutterRenderer.ViewportMetrics.class); - imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, deferredInsets); + // Set the initial insets imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, noneInsets); verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); @@ -2079,25 +2175,26 @@ public void ime_windowInsetsSync() { assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); + // Set the deferred insets and make a second call after the animation start which should be + // ignored imeSyncCallback.getAnimationCallback().onPrepare(animation); imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, deferredInsets); imeSyncCallback.getAnimationCallback().onStart(animation, null); - // Only the final state call is saved, extra calls are passed on. - imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, imeInsets2); + imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, altDeferredInsets); verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); - // No change, as deferredInset is stored to be passed in onEnd() assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); + // Apply the inset animation values imeSyncCallback.getAnimationCallback().onProgress(imeInsets0, animationList); verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); assertEquals(10, viewportMetricsCaptor.getValue().viewPaddingTop); - assertEquals(60, viewportMetricsCaptor.getValue().viewInsetBottom); + assertEquals(25, viewportMetricsCaptor.getValue().viewInsetBottom); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); imeSyncCallback.getAnimationCallback().onProgress(imeInsets1, animationList); @@ -2105,16 +2202,17 @@ public void ime_windowInsetsSync() { verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); assertEquals(10, viewportMetricsCaptor.getValue().viewPaddingTop); - assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom); // Cannot be negative + assertEquals(50, viewportMetricsCaptor.getValue().viewInsetBottom); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); + // At the end of the animation, the insets should match deferredInsets as the call to set + // altDeferredInsets should have been ignored imeSyncCallback.getAnimationCallback().onEnd(animation); verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); - // Values should be of deferredInsets, not imeInsets2 assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); assertEquals(10, viewportMetricsCaptor.getValue().viewPaddingTop); - assertEquals(200, viewportMetricsCaptor.getValue().viewInsetBottom); + assertEquals(100, viewportMetricsCaptor.getValue().viewInsetBottom); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); } From 30bfbf295f6856051813cf63c6609cbf63e74204 Mon Sep 17 00:00:00 2001 From: Jonathan Cole Date: Sat, 4 Mar 2023 17:46:30 +0000 Subject: [PATCH 3/4] Use WindowInsets.Type.navigationBars() --- .../ImeSyncDeferringInsetsCallback.java | 6 ++-- .../plugin/editing/TextInputPluginTest.java | 32 +++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/editing/ImeSyncDeferringInsetsCallback.java b/shell/platform/android/io/flutter/plugin/editing/ImeSyncDeferringInsetsCallback.java index 6c011ac2ccc99..a8976ef910c2d 100644 --- a/shell/platform/android/io/flutter/plugin/editing/ImeSyncDeferringInsetsCallback.java +++ b/shell/platform/android/io/flutter/plugin/editing/ImeSyncDeferringInsetsCallback.java @@ -129,13 +129,13 @@ public WindowInsets onProgress( // The IME insets include the height of the navigation bar. If the app isn't laid out behind // the navigation bar, this causes the IME insets will be too large during the animation. - // To fix this, we subtract the systemBars bottom inset if the system UI flags for laying out - // behind the navigation bar aren't present. + // To fix this, we subtract the navigationBars bottom inset if the system UI flags for laying + // out behind the navigation bar aren't present. int excludedInsets = 0; int systemUiFlags = view.getWindowSystemUiVisibility(); if ((systemUiFlags & View.SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION) == 0 && (systemUiFlags & View.SYSTEM_UI_FLAG_HIDE_NAVIGATION) == 0) { - excludedInsets = insets.getInsets(WindowInsets.Type.systemBars()).bottom; + excludedInsets = insets.getInsets(WindowInsets.Type.navigationBars()).bottom; } WindowInsets.Builder builder = new WindowInsets.Builder(lastWindowInsets); diff --git a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java index ee23124366072..b52d261a990d4 100644 --- a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java +++ b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java @@ -2029,7 +2029,7 @@ public void sendAppPrivateCommand_hasData() throws JSONException { @Test @TargetApi(30) @Config(sdk = 30) - public void ime_windowInsetsSync_notLaidOutBehindNavigation_excludesSystemBars() { + public void ime_windowInsetsSync_notLaidOutBehindNavigation_excludesNavigationBars() { FlutterView testView = spy(new FlutterView(Robolectric.setupActivity(Activity.class))); when(testView.getWindowSystemUiVisibility()).thenReturn(View.SYSTEM_UI_FLAG_LAYOUT_STABLE); @@ -2052,19 +2052,19 @@ public void ime_windowInsetsSync_notLaidOutBehindNavigation_excludesSystemBars() WindowInsets noneInsets = builder.build(); builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 25)); - builder.setInsets(WindowInsets.Type.systemBars(), Insets.of(10, 10, 10, 40)); + builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); WindowInsets imeInsets0 = builder.build(); builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 50)); - builder.setInsets(WindowInsets.Type.systemBars(), Insets.of(10, 10, 10, 40)); + builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); WindowInsets imeInsets1 = builder.build(); builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 100)); - builder.setInsets(WindowInsets.Type.systemBars(), Insets.of(10, 10, 10, 0)); + builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 0)); WindowInsets deferredInsets = builder.build(); builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 50)); - builder.setInsets(WindowInsets.Type.systemBars(), Insets.of(10, 10, 10, 40)); + builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); WindowInsets altDeferredInsets = builder.build(); ArgumentCaptor viewportMetricsCaptor = @@ -2097,7 +2097,7 @@ public void ime_windowInsetsSync_notLaidOutBehindNavigation_excludesSystemBars() verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); - assertEquals(10, viewportMetricsCaptor.getValue().viewPaddingTop); + assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); @@ -2105,7 +2105,7 @@ public void ime_windowInsetsSync_notLaidOutBehindNavigation_excludesSystemBars() verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); - assertEquals(10, viewportMetricsCaptor.getValue().viewPaddingTop); + assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(10, viewportMetricsCaptor.getValue().viewInsetBottom); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); @@ -2115,7 +2115,7 @@ public void ime_windowInsetsSync_notLaidOutBehindNavigation_excludesSystemBars() verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); - assertEquals(10, viewportMetricsCaptor.getValue().viewPaddingTop); + assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(100, viewportMetricsCaptor.getValue().viewInsetBottom); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); } @@ -2123,7 +2123,7 @@ public void ime_windowInsetsSync_notLaidOutBehindNavigation_excludesSystemBars() @Test @TargetApi(30) @Config(sdk = 30) - public void ime_windowInsetsSync_laidOutBehindNavigation_includesSystemBars() { + public void ime_windowInsetsSync_laidOutBehindNavigation_includesNavigationBars() { FlutterView testView = spy(new FlutterView(Robolectric.setupActivity(Activity.class))); when(testView.getWindowSystemUiVisibility()) .thenReturn( @@ -2148,19 +2148,19 @@ public void ime_windowInsetsSync_laidOutBehindNavigation_includesSystemBars() { WindowInsets noneInsets = builder.build(); builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 25)); - builder.setInsets(WindowInsets.Type.systemBars(), Insets.of(10, 10, 10, 40)); + builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); WindowInsets imeInsets0 = builder.build(); builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 50)); - builder.setInsets(WindowInsets.Type.systemBars(), Insets.of(10, 10, 10, 40)); + builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); WindowInsets imeInsets1 = builder.build(); builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 100)); - builder.setInsets(WindowInsets.Type.systemBars(), Insets.of(10, 10, 10, 0)); + builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 0)); WindowInsets deferredInsets = builder.build(); builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 50)); - builder.setInsets(WindowInsets.Type.systemBars(), Insets.of(10, 10, 10, 40)); + builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); WindowInsets altDeferredInsets = builder.build(); ArgumentCaptor viewportMetricsCaptor = @@ -2193,7 +2193,7 @@ public void ime_windowInsetsSync_laidOutBehindNavigation_includesSystemBars() { verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); - assertEquals(10, viewportMetricsCaptor.getValue().viewPaddingTop); + assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(25, viewportMetricsCaptor.getValue().viewInsetBottom); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); @@ -2201,7 +2201,7 @@ public void ime_windowInsetsSync_laidOutBehindNavigation_includesSystemBars() { verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); - assertEquals(10, viewportMetricsCaptor.getValue().viewPaddingTop); + assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(50, viewportMetricsCaptor.getValue().viewInsetBottom); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); @@ -2211,7 +2211,7 @@ public void ime_windowInsetsSync_laidOutBehindNavigation_includesSystemBars() { verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); - assertEquals(10, viewportMetricsCaptor.getValue().viewPaddingTop); + assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(100, viewportMetricsCaptor.getValue().viewInsetBottom); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); } From 77ec64e1a93948564a08bf5dfefcfba6f046db3b Mon Sep 17 00:00:00 2001 From: Jonathan Cole Date: Thu, 20 Apr 2023 21:52:13 +0100 Subject: [PATCH 4/4] Test clarity improvements and comment fix --- .../ImeSyncDeferringInsetsCallback.java | 2 +- .../plugin/editing/TextInputPluginTest.java | 136 +++++++----------- 2 files changed, 51 insertions(+), 87 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/editing/ImeSyncDeferringInsetsCallback.java b/shell/platform/android/io/flutter/plugin/editing/ImeSyncDeferringInsetsCallback.java index a8976ef910c2d..fc9a72e494170 100644 --- a/shell/platform/android/io/flutter/plugin/editing/ImeSyncDeferringInsetsCallback.java +++ b/shell/platform/android/io/flutter/plugin/editing/ImeSyncDeferringInsetsCallback.java @@ -128,7 +128,7 @@ public WindowInsets onProgress( } // The IME insets include the height of the navigation bar. If the app isn't laid out behind - // the navigation bar, this causes the IME insets will be too large during the animation. + // the navigation bar, this causes the IME insets to be too large during the animation. // To fix this, we subtract the navigationBars bottom inset if the system UI flags for laying // out behind the navigation bar aren't present. int excludedInsets = 0; diff --git a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java index b52d261a990d4..b45fd05fa4805 100644 --- a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java +++ b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java @@ -2048,76 +2048,58 @@ public void ime_windowInsetsSync_notLaidOutBehindNavigation_excludesNavigationBa List animationList = new ArrayList(); animationList.add(animation); + ArgumentCaptor viewportMetricsCaptor = + ArgumentCaptor.forClass(FlutterRenderer.ViewportMetrics.class); + WindowInsets.Builder builder = new WindowInsets.Builder(); - WindowInsets noneInsets = builder.build(); - builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 25)); - builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); - WindowInsets imeInsets0 = builder.build(); + // Set the initial insets and verify that they were set and the bottom view inset is correct + imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, builder.build()); - builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 50)); - builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); - WindowInsets imeInsets1 = builder.build(); + verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); + assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom); + // Call onPrepare and set the lastWindowInsets - these should be stored for the end of the + // animation instead of being applied immediately + imeSyncCallback.getAnimationCallback().onPrepare(animation); builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 100)); builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 0)); - WindowInsets deferredInsets = builder.build(); - - builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 50)); - builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); - WindowInsets altDeferredInsets = builder.build(); - - ArgumentCaptor viewportMetricsCaptor = - ArgumentCaptor.forClass(FlutterRenderer.ViewportMetrics.class); - - // Set the initial insets - imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, noneInsets); + imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, builder.build()); verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); - // Set the deferred insets and make a second call after the animation start which should be - // ignored - imeSyncCallback.getAnimationCallback().onPrepare(animation); - imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, deferredInsets); + // Call onStart and apply new insets - these should be ignored completely imeSyncCallback.getAnimationCallback().onStart(animation, null); - imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, altDeferredInsets); + builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 50)); + builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); + imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, builder.build()); verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); - // Apply the inset animation values - imeSyncCallback.getAnimationCallback().onProgress(imeInsets0, animationList); + // Progress the animation and ensure that the navigation bar insets have been subtracted + // from the IME insets + builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 25)); + builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); + imeSyncCallback.getAnimationCallback().onProgress(builder.build(), animationList); verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); - imeSyncCallback.getAnimationCallback().onProgress(imeInsets1, animationList); + builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 50)); + builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); + imeSyncCallback.getAnimationCallback().onProgress(builder.build(), animationList); verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(10, viewportMetricsCaptor.getValue().viewInsetBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); - // At the end of the animation, the insets should match deferredInsets as the call to set - // altDeferredInsets should have been ignored + // End the animation and ensure that the bottom insets match the lastWindowInsets that we set + // during onPrepare imeSyncCallback.getAnimationCallback().onEnd(animation); verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(100, viewportMetricsCaptor.getValue().viewInsetBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); } @Test @@ -2144,76 +2126,58 @@ public void ime_windowInsetsSync_laidOutBehindNavigation_includesNavigationBars( List animationList = new ArrayList(); animationList.add(animation); + ArgumentCaptor viewportMetricsCaptor = + ArgumentCaptor.forClass(FlutterRenderer.ViewportMetrics.class); + WindowInsets.Builder builder = new WindowInsets.Builder(); - WindowInsets noneInsets = builder.build(); - builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 25)); - builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); - WindowInsets imeInsets0 = builder.build(); + // Set the initial insets and verify that they were set and the bottom view inset is correct + imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, builder.build()); - builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 50)); - builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); - WindowInsets imeInsets1 = builder.build(); + verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); + assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom); + // Call onPrepare and set the lastWindowInsets - these should be stored for the end of the + // animation instead of being applied immediately + imeSyncCallback.getAnimationCallback().onPrepare(animation); builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 100)); builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 0)); - WindowInsets deferredInsets = builder.build(); - - builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 50)); - builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); - WindowInsets altDeferredInsets = builder.build(); - - ArgumentCaptor viewportMetricsCaptor = - ArgumentCaptor.forClass(FlutterRenderer.ViewportMetrics.class); - - // Set the initial insets - imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, noneInsets); + imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, builder.build()); verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); - // Set the deferred insets and make a second call after the animation start which should be - // ignored - imeSyncCallback.getAnimationCallback().onPrepare(animation); - imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, deferredInsets); + // Call onStart and apply new insets - these should be ignored completely imeSyncCallback.getAnimationCallback().onStart(animation, null); - imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, altDeferredInsets); + builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 50)); + builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); + imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, builder.build()); verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); - // Apply the inset animation values - imeSyncCallback.getAnimationCallback().onProgress(imeInsets0, animationList); + // Progress the animation and ensure that the navigation bar insets have not been + // subtracted from the IME insets + builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 25)); + builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); + imeSyncCallback.getAnimationCallback().onProgress(builder.build(), animationList); verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(25, viewportMetricsCaptor.getValue().viewInsetBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); - imeSyncCallback.getAnimationCallback().onProgress(imeInsets1, animationList); + builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 50)); + builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 40)); + imeSyncCallback.getAnimationCallback().onProgress(builder.build(), animationList); verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(50, viewportMetricsCaptor.getValue().viewInsetBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); - // At the end of the animation, the insets should match deferredInsets as the call to set - // altDeferredInsets should have been ignored + // End the animation and ensure that the bottom insets match the lastWindowInsets that we set + // during onPrepare imeSyncCallback.getAnimationCallback().onEnd(animation); verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture()); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop); assertEquals(100, viewportMetricsCaptor.getValue().viewInsetBottom); - assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop); } interface EventHandler {