Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Android] Fix incorrect viewInsets during keyboard animation with EdgeToEdge #39391

Merged
merged 7 commits into from
May 8, 2023
Merged

[Android] Fix incorrect viewInsets during keyboard animation with EdgeToEdge #39391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6945789
Fix navigation bar insets incorrectly being subtracted from keyboard …
JonathanPeterCole Feb 4, 2023
e63b10f
Update tests
JonathanPeterCole Feb 4, 2023
044cb13
Merge branch 'main' into edge-to-edge-keyboard-animation-fix
JonathanPeterCole Mar 4, 2023
30bfbf2
Use WindowInsets.Type.navigationBars()
JonathanPeterCole Mar 4, 2023
77ec64e
Test clarity improvements and comment fix
JonathanPeterCole Apr 20, 2023
57c2a5f
Merge branch 'main' into edge-to-edge-keyboard-animation-fix
JonathanPeterCole Apr 20, 2023
1b3e603
Merge branch 'main' into edge-to-edge-keyboard-animation-fix
reidbaker May 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 16 additions & 21 deletions shell/platform/android/io/flutter/plugin/editing/ImeSyncDeferringInsetsCallback.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand Down Expand Up @@ -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 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;
int systemUiFlags = view.getWindowSystemUiVisibility();
if ((systemUiFlags & View.SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION) == 0
Copy link
Contributor

@reidbaker reidbaker May 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without committing you to building it do you think we need to do a similar evaluation for status bar and the top insets?

Copy link
Contributor Author

@JonathanPeterCole JonathanPeterCole May 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that'll be necessary as the status bar visibility and top insets didn't affect the keyboard animation in my testing.

Copy link
Contributor

@reidbaker reidbaker May 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it would impact any animation coming from the top like keyboards impact animations coming from the bottom. Right? If for example the status bar was present our code would behave differently than if status bar was overlayed. I am thinking of flutter/flutter#118761

Copy link
Contributor Author

@JonathanPeterCole JonathanPeterCole May 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah interesting, I'm not sure on this one but WindowInsetsController.BEHAVIOR_SHOW_TRANSIENT_BARS_BY_SWIPE is one of the new APIs introduced in API 30, so I wonder if it's a conflict with the deprecated system UI flags currently in use, rather than an issue with keyboard animation.

Copy link
Contributor

@reidbaker reidbaker May 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gmackall this is something to keep in mind.

&& (systemUiFlags & View.SYSTEM_UI_FLAG_HIDE_NAVIGATION) == 0) {
excludedInsets = insets.getInsets(WindowInsets.Type.navigationBars()).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
Expand Down
15 changes: 1 addition & 14 deletions shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}

Expand Down
146 changes: 104 additions & 42 deletions shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_excludesNavigationBars() {
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));
Expand All @@ -2046,76 +2048,136 @@ public void ime_windowInsetsSync() {
List<WindowInsetsAnimation> animationList = new ArrayList();
animationList.add(animation);

ArgumentCaptor<FlutterRenderer.ViewportMetrics> viewportMetricsCaptor =
ArgumentCaptor.forClass(FlutterRenderer.ViewportMetrics.class);

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.
// Set the initial insets and verify that they were set and the bottom view inset is correct
imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, 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(10, 10, 10, 40));
WindowInsets imeInsets0 = builder.build();
builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 0));
imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, builder.build());

verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture());
assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom);

// Call onStart and apply new insets - these should be ignored completely
imeSyncCallback.getAnimationCallback().onStart(animation, null);
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());

builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 30));
builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(10, 10, 10, 40));
WindowInsets imeInsets1 = builder.build();
verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture());
assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom);

// 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().viewInsetBottom);

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.navigationBars(), Insets.of(0, 0, 0, 40));
imeSyncCallback.getAnimationCallback().onProgress(builder.build(), animationList);

verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture());
assertEquals(10, viewportMetricsCaptor.getValue().viewInsetBottom);

// 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(100, viewportMetricsCaptor.getValue().viewInsetBottom);
}

@Test
@TargetApi(30)
@Config(sdk = 30)
public void ime_windowInsetsSync_laidOutBehindNavigation_includesNavigationBars() {
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);

builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 200));
builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(10, 10, 10, 0));
WindowInsets deferredInsets = builder.build();
WindowInsetsAnimation animation = mock(WindowInsetsAnimation.class);
when(animation.getTypeMask()).thenReturn(WindowInsets.Type.ime());

List<WindowInsetsAnimation> animationList = new ArrayList();
animationList.add(animation);

ArgumentCaptor<FlutterRenderer.ViewportMetrics> viewportMetricsCaptor =
ArgumentCaptor.forClass(FlutterRenderer.ViewportMetrics.class);

imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, deferredInsets);
imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, noneInsets);
WindowInsets.Builder builder = new WindowInsets.Builder();

// Set the initial insets and verify that they were set and the bottom view inset is correct
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);

// 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);
imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, deferredInsets);
builder.setInsets(WindowInsets.Type.ime(), Insets.of(0, 0, 0, 100));
builder.setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, 0));
imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, builder.build());

verify(flutterRenderer, atLeast(1)).setViewportMetrics(viewportMetricsCaptor.capture());
assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom);

// Call onStart and apply new insets - these should be ignored completely
imeSyncCallback.getAnimationCallback().onStart(animation, null);
// Only the final state call is saved, extra calls are passed on.
imeSyncCallback.getInsetsListener().onApplyWindowInsets(testView, imeInsets2);
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());
// 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);

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(10, viewportMetricsCaptor.getValue().viewPaddingTop);
assertEquals(60, viewportMetricsCaptor.getValue().viewInsetBottom);
assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop);
assertEquals(25, viewportMetricsCaptor.getValue().viewInsetBottom);

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(10, viewportMetricsCaptor.getValue().viewPaddingTop);
assertEquals(0, viewportMetricsCaptor.getValue().viewInsetBottom); // Cannot be negative
assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop);
assertEquals(50, viewportMetricsCaptor.getValue().viewInsetBottom);

// 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());
// Values should be of deferredInsets, not imeInsets2
assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingBottom);
assertEquals(10, viewportMetricsCaptor.getValue().viewPaddingTop);
assertEquals(200, viewportMetricsCaptor.getValue().viewInsetBottom);
assertEquals(0, viewportMetricsCaptor.getValue().viewInsetTop);
assertEquals(100, viewportMetricsCaptor.getValue().viewInsetBottom);
}

interface EventHandler {
Expand Down