-
Notifications
You must be signed in to change notification settings - Fork 6k
Add fullscreen padding workarounds to v2 android embedding #18193
Changes from all commits
4e84704
24050a6
cc90f62
f9a2d84
6a02e91
8a14d06
4b1a3ae
f6880ea
7e81902
21311d5
58e0953
23add3e
1af9e58
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ | |
import android.view.View; | ||
import android.view.ViewStructure; | ||
import android.view.WindowInsets; | ||
import android.view.WindowManager; | ||
import android.view.accessibility.AccessibilityManager; | ||
import android.view.accessibility.AccessibilityNodeProvider; | ||
import android.view.autofill.AutofillValue; | ||
|
@@ -535,21 +536,24 @@ protected void onSizeChanged(int width, int height, int oldWidth, int oldHeight) | |
// android may decide to place the software navigation bars on the side. When the nav | ||
// bar is hidden, the reported insets should be removed to prevent extra useless space | ||
// on the sides. | ||
enum ZeroSides { | ||
private enum ZeroSides { | ||
NONE, | ||
LEFT, | ||
RIGHT, | ||
BOTH | ||
} | ||
|
||
ZeroSides calculateShouldZeroSides() { | ||
private ZeroSides calculateShouldZeroSides() { | ||
// We get both orientation and rotation because rotation is all 4 | ||
// rotations relative to default rotation while orientation is portrait | ||
// or landscape. By combining both, we can obtain a more precise measure | ||
// of the rotation. | ||
Activity activity = (Activity) getContext(); | ||
int orientation = activity.getResources().getConfiguration().orientation; | ||
int rotation = activity.getWindowManager().getDefaultDisplay().getRotation(); | ||
Context context = getContext(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This avoids casting to Activity which can be unsafe |
||
int orientation = context.getResources().getConfiguration().orientation; | ||
int rotation = | ||
((WindowManager) context.getSystemService(Context.WINDOW_SERVICE)) | ||
.getDefaultDisplay() | ||
.getRotation(); | ||
|
||
if (orientation == Configuration.ORIENTATION_LANDSCAPE) { | ||
if (rotation == Surface.ROTATION_90) { | ||
|
@@ -568,7 +572,7 @@ else if (rotation == Surface.ROTATION_0 || rotation == Surface.ROTATION_180) { | |
return ZeroSides.NONE; | ||
} | ||
|
||
// TODO(garyq): Use clean ways to detect keyboard instead of heuristics if possible | ||
// TODO(garyq): Use new Android R getInsets API | ||
// TODO(garyq): The keyboard detection may interact strangely with | ||
// https://github.com/flutter/flutter/issues/22061 | ||
|
||
|
@@ -578,7 +582,7 @@ else if (rotation == Surface.ROTATION_0 || rotation == Surface.ROTATION_180) { | |
// can be used. | ||
@TargetApi(20) | ||
@RequiresApi(20) | ||
int calculateBottomKeyboardInset(WindowInsets insets) { | ||
private int guessBottomKeyboardInset(WindowInsets insets) { | ||
int screenHeight = getRootView().getHeight(); | ||
// Magic number due to this being a heuristic. This should be replaced, but we have not | ||
// found a clean way to do it yet (Sept. 2018) | ||
|
@@ -632,7 +636,7 @@ public final WindowInsets onApplyWindowInsets(WindowInsets insets) { | |
// the navbar padding should always be provided. | ||
mMetrics.physicalViewInsetBottom = | ||
navigationBarHidden | ||
? calculateBottomKeyboardInset(insets) | ||
? guessBottomKeyboardInset(insets) | ||
: insets.getSystemWindowInsetBottom(); | ||
mMetrics.physicalViewInsetLeft = 0; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ | |
import android.view.View; | ||
import android.view.ViewGroup; | ||
import android.view.WindowInsets; | ||
import android.view.WindowManager; | ||
import io.flutter.embedding.engine.FlutterEngine; | ||
import io.flutter.embedding.engine.FlutterJNI; | ||
import io.flutter.embedding.engine.loader.FlutterLoader; | ||
|
@@ -33,9 +34,11 @@ | |
import org.mockito.stubbing.Answer; | ||
import org.robolectric.RobolectricTestRunner; | ||
import org.robolectric.RuntimeEnvironment; | ||
import org.robolectric.Shadows; | ||
import org.robolectric.annotation.Config; | ||
import org.robolectric.annotation.Implementation; | ||
import org.robolectric.annotation.Implements; | ||
import org.robolectric.shadows.ShadowDisplay; | ||
|
||
// TODO(xster): we have 2 versions of robolectric Android shadows in | ||
// shell/platform/android/embedding_bundle/build.gradle. Remove the older | ||
|
@@ -278,6 +281,100 @@ public void reportSystemInsetWhenNotFullscreen() { | |
assertEquals(100, viewportMetricsCaptor.getValue().paddingRight); | ||
} | ||
|
||
@Test | ||
public void systemInsetHandlesFullscreenNavbarRight() { | ||
RuntimeEnvironment.setQualifiers("+land"); | ||
FlutterView flutterView = spy(new FlutterView(RuntimeEnvironment.systemContext)); | ||
ShadowDisplay display = | ||
Shadows.shadowOf( | ||
((WindowManager) | ||
RuntimeEnvironment.systemContext.getSystemService(Context.WINDOW_SERVICE)) | ||
.getDefaultDisplay()); | ||
display.setRotation(1); | ||
assertEquals(0, flutterView.getSystemUiVisibility()); | ||
when(flutterView.getWindowSystemUiVisibility()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are you sure this works? You're altering the response of the spy instance but I'm assuming FlutterView is calling getWindowSystemUiVisibility() against itself internally and not the spy. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like it does. It is getting the correct values internally, and executing the right branches of code. Removing this breaks it. |
||
.thenReturn(View.SYSTEM_UI_FLAG_FULLSCREEN | View.SYSTEM_UI_FLAG_HIDE_NAVIGATION); | ||
when(flutterView.getContext()).thenReturn(RuntimeEnvironment.systemContext); | ||
|
||
FlutterEngine flutterEngine = | ||
spy(new FlutterEngine(RuntimeEnvironment.application, mockFlutterLoader, mockFlutterJni)); | ||
FlutterRenderer flutterRenderer = spy(new FlutterRenderer(mockFlutterJni)); | ||
when(flutterEngine.getRenderer()).thenReturn(flutterRenderer); | ||
|
||
// When we attach a new FlutterView to the engine without any system insets, | ||
// the viewport metrics default to 0. | ||
flutterView.attachToFlutterEngine(flutterEngine); | ||
ArgumentCaptor<FlutterRenderer.ViewportMetrics> viewportMetricsCaptor = | ||
ArgumentCaptor.forClass(FlutterRenderer.ViewportMetrics.class); | ||
verify(flutterRenderer).setViewportMetrics(viewportMetricsCaptor.capture()); | ||
assertEquals(0, viewportMetricsCaptor.getValue().paddingTop); | ||
|
||
// Then we simulate the system applying a window inset. | ||
WindowInsets windowInsets = mock(WindowInsets.class); | ||
when(windowInsets.getSystemWindowInsetTop()).thenReturn(100); | ||
when(windowInsets.getSystemWindowInsetBottom()).thenReturn(100); | ||
when(windowInsets.getSystemWindowInsetLeft()).thenReturn(100); | ||
when(windowInsets.getSystemWindowInsetRight()).thenReturn(100); | ||
|
||
flutterView.onApplyWindowInsets(windowInsets); | ||
|
||
verify(flutterRenderer, times(2)).setViewportMetrics(viewportMetricsCaptor.capture()); | ||
// Top padding is removed due to full screen. | ||
assertEquals(0, viewportMetricsCaptor.getValue().paddingTop); | ||
// Padding bottom is always 0. | ||
assertEquals(0, viewportMetricsCaptor.getValue().paddingBottom); | ||
assertEquals(100, viewportMetricsCaptor.getValue().paddingLeft); | ||
// Right padding is zero because the rotation is 270deg | ||
assertEquals(0, viewportMetricsCaptor.getValue().paddingRight); | ||
} | ||
|
||
@Test | ||
public void systemInsetHandlesFullscreenNavbarLeft() { | ||
RuntimeEnvironment.setQualifiers("+land"); | ||
FlutterView flutterView = spy(new FlutterView(RuntimeEnvironment.systemContext)); | ||
ShadowDisplay display = | ||
Shadows.shadowOf( | ||
((WindowManager) | ||
RuntimeEnvironment.systemContext.getSystemService(Context.WINDOW_SERVICE)) | ||
.getDefaultDisplay()); | ||
display.setRotation(3); | ||
assertEquals(0, flutterView.getSystemUiVisibility()); | ||
when(flutterView.getWindowSystemUiVisibility()) | ||
.thenReturn(View.SYSTEM_UI_FLAG_FULLSCREEN | View.SYSTEM_UI_FLAG_HIDE_NAVIGATION); | ||
when(flutterView.getContext()).thenReturn(RuntimeEnvironment.systemContext); | ||
|
||
FlutterEngine flutterEngine = | ||
spy(new FlutterEngine(RuntimeEnvironment.application, mockFlutterLoader, mockFlutterJni)); | ||
FlutterRenderer flutterRenderer = spy(new FlutterRenderer(mockFlutterJni)); | ||
when(flutterEngine.getRenderer()).thenReturn(flutterRenderer); | ||
|
||
// When we attach a new FlutterView to the engine without any system insets, | ||
// the viewport metrics default to 0. | ||
flutterView.attachToFlutterEngine(flutterEngine); | ||
ArgumentCaptor<FlutterRenderer.ViewportMetrics> viewportMetricsCaptor = | ||
ArgumentCaptor.forClass(FlutterRenderer.ViewportMetrics.class); | ||
verify(flutterRenderer).setViewportMetrics(viewportMetricsCaptor.capture()); | ||
assertEquals(0, viewportMetricsCaptor.getValue().paddingTop); | ||
|
||
// Then we simulate the system applying a window inset. | ||
WindowInsets windowInsets = mock(WindowInsets.class); | ||
when(windowInsets.getSystemWindowInsetTop()).thenReturn(100); | ||
when(windowInsets.getSystemWindowInsetBottom()).thenReturn(100); | ||
when(windowInsets.getSystemWindowInsetLeft()).thenReturn(100); | ||
when(windowInsets.getSystemWindowInsetRight()).thenReturn(100); | ||
|
||
flutterView.onApplyWindowInsets(windowInsets); | ||
|
||
verify(flutterRenderer, times(2)).setViewportMetrics(viewportMetricsCaptor.capture()); | ||
// Top padding is removed due to full screen. | ||
assertEquals(0, viewportMetricsCaptor.getValue().paddingTop); | ||
// Padding bottom is always 0. | ||
assertEquals(0, viewportMetricsCaptor.getValue().paddingBottom); | ||
// Left padding is zero because the rotation is 270deg | ||
assertEquals(0, viewportMetricsCaptor.getValue().paddingLeft); | ||
assertEquals(100, viewportMetricsCaptor.getValue().paddingRight); | ||
} | ||
|
||
/* | ||
* A custom shadow that reports fullscreen flag for system UI visibility | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mirror an Android API? If so, consider adding a See: link or identifier names to help readers link the Flutter/Android concepts/APIs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This information seems to be intentionally hidden from us to prevent people from locking users out of their phone by preventing touches to the navbar