Add MediaQuery.systemGestureInsets to support Android Q by shihaohong · Pull Request #37416 · flutter/flutter

shihaohong · 2019-08-01T19:17:57Z

Description:

This passes gestural navigation inset information to the MediaQuery.systemGestureInsets. This becomes necessary as Flutter app developers want to take advantage of their entire screen with full gestural navigation mode coming with Q. This allows Flutter developers to determine the minimum distance from each edge needed for any swipe-able widgets like sliders.

Related Issues

Related to #32748, #34678
Depends on flutter/engine#10413

Tests

I added the following tests:

Updated existing tests such as copyWith and defaulting to source.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

Yes, this is a breaking change (Please read Handling breaking changes). Replace this with a link to the e-mail where you asked for input on this proposed change.
No, this is not a breaking change.

shihaohong · 2019-08-07T22:37:30Z

I'm currently working on the engine counterpart of this PR, so until that lands, this PR will remain a work in progress!

shihaohong · 2019-08-14T23:07:58Z

I'm closing this for now, since this is not actionable given that we're blocked on the engine PR. I will create a new PR once the engine PR is completed.

…stem-gesture-navigation

goderbauer

LGTM

goderbauer · 2019-08-20T23:20:47Z

packages/flutter/lib/src/widgets/media_query.dart

  ///   property and how it relates to [padding] and [viewInsets].
  final EdgeInsets viewPadding;

+  /// The parts of the display that contain system gestures, typically swipes


"contains" is kinds strange. Isn't this just the area where the system may detect certain gestures, handle them internally, and not forward those to your app?

I will update this to

/// The parts of the display where the system may be detecting particular /// gestures. /// /// This value is typically a drag gesture from an edge of the screen.

Let me know if there is any additional improvement/clarification we could make

It's important to make this extra clear and part of that will be explaining exactly what AndroidQ does. So:

The areas along the edges of the display where the system consumes certain input events and blocks delivery of those events to the app.

Starting with Android Q, simple swipe gestures that start within the [systemGestureInsets] areas are used by the system for page navigation and are not delivered to the app. Taps and swipe gestures that begin with a long-press are delivered to the app but simple press-drag-release swipe gestures which begin within the area defined by [systemGestureInsets] are not.

Apps should avoid locating gesture detectors within the system gesture insets area. There's no reason to avoid putting visuals within this area.

There's a little nuance to this when I was testing it yesterday... The swipe gesture can still be registered by the app. I've gotten a drawer to open on a simple swipe, but it simultaneously triggers the back navigation arrow to appear. However, this is not consistent and sometimes, the drawer will appear and almost immediately disappear once the back navigation arrow starts to appear.

I'll try to share a gif, but I think the general idea is that there will be conflicts in how simple drag gestures are handled in these inset areas and that devs should just avoid it anyway.

Did you have a chance to just place a Listener [1] in this area and see what events you actually get in Flutter?

[1] https://master-api.flutter.dev/flutter/widgets/Listener-class.html

All this to say, maybe updating the wording to "simple swipe gestures that start within the [systemGestureInsets] areas are used by the system for page navigation and ~~are~~ may not be delivered to the app"

Sounds OK to me. We can tighten up the wording in the future, if we need to.

@goderbauer I just played with the Listener, and it seems like it lets all gestures through and once you start doing a simple horizontal swipe, it sends a cancel event and begins the system gesture.

Oddly, if you do a diagonal swipe from the edge (ie. top-left to bottom-right or bottom-left to top-right), that seems to never trigger the system gesture. I decided to try this after reading about it as a "workaround" on some articles on the internet.

goderbauer · 2019-08-20T23:22:02Z

packages/flutter/lib/src/widgets/media_query.dart


+  /// The parts of the display that contain system gestures, typically swipes
+  /// from the edge of the screen for navigation.
+  ///


Can you add a little more information here, e.g. that certain gestures in these areas my not be forwarded to the app and elements placed here may therefore not respond to them.

Sounds good, I did the following:

/// This value is typically a drag gesture from an edge of the screen. /// Gestures in these areas may not be forwarded to the app or may /// be canceled by the system. Therefore, elements placed in these areas may /// not properly respond to them.

You might be able to reuse a version of #37416 (comment) here.

goderbauer · 2019-08-21T17:46:19Z

packages/flutter/lib/src/widgets/media_query.dart


-  /// The parts of the display that contain system gestures, typically swipes
-  /// from the edge of the screen for navigation.
+  /// The parts of the display where the system may be detecting particular


Maybe:

The parts of the display where the system may be detecting certain gestures for system actions.

Those gestures are typically drags from the edge of the screen to perform system actions like opening the app drawer or the notification tray. Gestures reserved for system actions may not be forwarded to the app when performed in the area specified by these insets . Therefore, elements placed in these areas may not properly respond to them.

I tweaked the example a little (replaced opening the app drawer with back navigation), but incorporated everything else in your suggestion

goderbauer · 2019-08-21T17:47:22Z

packages/flutter/lib/src/widgets/media_query.dart

+  /// This value is typically a drag gesture from an edge of the screen.
+  /// Gestures in these areas may not be forwarded to the app or may
+  /// be canceled by the system. Therefore, elements placed in these areas may
+  /// not properly respond to them.


We should say clearly on what systems this is used (e.g. Android starting with version Q)?

That's a great idea, I just added it.

HansMuller · 2019-08-21T19:43:00Z

packages/flutter/lib/src/widgets/media_query.dart

+  ///
+  /// {@tool snippet --template=stateful_widget_material}
+  ///
+  /// When using Android Q with full gestural navigation, use


For apps that might be deployed on Android Q devices with full ...

HansMuller · 2019-08-21T19:44:26Z

packages/flutter/lib/src/widgets/media_query.dart

+  /// Widget build(BuildContext context) {
+  ///   EdgeInsets systemGestureInsets = MediaQuery.of(context).systemGestureInsets;
+  ///   return Scaffold(
+  ///     appBar: AppBar(title: Text('Sample Gesture Nav')),


for the title, maybe 'Pad Slider to avoid systemGestureInsets'

HansMuller · 2019-08-21T19:48:09Z

packages/flutter/lib/src/widgets/media_query.dart

+  ///   EdgeInsets systemGestureInsets = MediaQuery.of(context).systemGestureInsets;
+  ///   return Scaffold(
+  ///     appBar: AppBar(title: Text('Sample Gesture Nav')),
+  ///     body: Padding(


Need a comment here to explain that, by default, the slider expands to fill the available width.

Not clear why we're padding top/bottom (we're assuming something about systemGestureInsets?). Maybe:

Container( alignment: Alignment.center, padding: systemGestureInsets, // only left,right padding is likely to be needed child: Slider... ),

I mainly pad the top/bottom to make it look cleaner in the sample app. I'll just set top and bottom to 0 for the sake of the sample to avoid confusion, which seems like a worthy tradeoff.

I think setting it to systemGestureInsets altogether might be a little confusing since there are systemGestureInsets on the top and bottom, but for this example, the Slider is in the body of the Scaffold and does not need to be padded by those values.

HansMuller · 2019-08-21T19:52:32Z

packages/flutter/test/widgets/media_query_test.dart

    expect(copied.padding, const EdgeInsets.all(9.10938));
    expect(copied.viewPadding, const EdgeInsets.all(11.24031));
    expect(copied.viewInsets, const EdgeInsets.all(1.67262));
+    expect(copied.systemGestureInsets, const EdgeInsets.all(1.5556));


Separate PR: define these double values with a consts and explain why we're using strange values.

Filed #39012

HansMuller

LGTM

HansMuller · 2019-08-21T23:51:38Z

packages/flutter/lib/src/widgets/media_query.dart

+  ///   return Scaffold(
+  ///     appBar: AppBar(title: Text('Pad Slider to avoid systemGestureInsets')),
+  ///     body: Padding(
+  ///       padding: EdgeInsets.fromLTRB( // only left and right padding are likely to be needed


In this case you could use EdgeInsets.only

HansMuller · 2019-08-21T23:53:04Z

packages/flutter/lib/src/widgets/media_query.dart

+  ///
+  /// For apps that might be deployed on Android Q devices with full gesture
+  /// navigation enabled, use [MediaQuery.systemGestureInsets] with [Padding]
+  /// to avoid overlapping a [Slider] with system gesture navigation.


To avoid having the left and right edges of the slider appear within the area reserved for system gesture navigation.

HansMuller · 2019-08-21T23:54:02Z

packages/flutter/lib/src/widgets/media_query.dart

+  /// and may not be delivered to the app. Taps and swipe gestures that begin
+  /// with a long-press are delivered to the app, but simple press-drag-release
+  /// swipe gestures which begin within the area defined by [systemGestureInsets]
+  /// may not.


may not => may not be

HansMuller · 2019-08-21T23:55:15Z

packages/flutter/lib/src/widgets/media_query.dart

+  /// may not.
+  ///
+  /// Apps should avoid locating gesture detectors within the system gesture
+  /// insets area. There is no reason to avoid putting visual elements within


[my fault] "There is no reason" is probably overstating it. Maybe: Apps should feel free to put visual elements within

Add MediaQuery.systemGestureInsets

5f8dbb7

googlebot added the cla: yes label Aug 1, 2019

shihaohong changed the title ~~(WIP) Add MediaQuery.systemGestureInsets~~ (WIP) Add MediaQuery.systemGestureInsets to support Android Q Full Gestural Navigation Aug 1, 2019

shihaohong requested a review from HansMuller August 1, 2019 19:19

shihaohong closed this Aug 14, 2019

shihaohong reopened this Aug 20, 2019

shihaohong removed the work in progress; do not review label Aug 20, 2019

shihaohong changed the title ~~(WIP) Add MediaQuery.systemGestureInsets to support Android Q Full Gestural Navigation~~ Add MediaQuery.systemGestureInsets to support Android Q Full Gestural Navigation Aug 20, 2019

shihaohong added 2 commits August 20, 2019 11:14

Merge with upstream/master

4a34936

Resolve missed merge conflicts in media_query.dart

7dc2ce7

shihaohong changed the title ~~Add MediaQuery.systemGestureInsets to support Android Q Full Gestural Navigation~~ Add MediaQuery.systemGestureInsets to support Android Q Aug 20, 2019

shihaohong changed the title ~~Add MediaQuery.systemGestureInsets to support Android Q~~ (WIP) Add MediaQuery.systemGestureInsets to support Android Q Aug 20, 2019

shihaohong added the work in progress; do not review label Aug 20, 2019

shihaohong added 2 commits August 20, 2019 13:35

Correct sample case for systemGestureInsets

c6a925e

Remove accidental trailing } in dart docs

600c3e2

shihaohong changed the title ~~(WIP) Add MediaQuery.systemGestureInsets to support Android Q~~ Add MediaQuery.systemGestureInsets to support Android Q Aug 20, 2019

shihaohong removed the work in progress; do not review label Aug 20, 2019

shihaohong added 2 commits August 20, 2019 14:40

Merge branch 'master' of github.com:flutter/flutter into android-q-sy…

5348b58

…stem-gesture-navigation

Add systemGestureInsets property to TestWindow

9eb549b

shihaohong requested a review from goderbauer August 20, 2019 22:17

Remove duplicate nested scaffolds in api doc sample

13c647b

goderbauer approved these changes Aug 20, 2019

View reviewed changes

Improve MediaQueryData.systemGestureInsets documentation

648bfbe

goderbauer reviewed Aug 21, 2019

View reviewed changes

shihaohong added 2 commits August 21, 2019 12:36

Revised API docs per feedback

7d95bb5

Clarify systemGestureInsets role in Android Q

5c92ff8

HansMuller reviewed Aug 21, 2019

View reviewed changes

Improve documentation further after more code review

ffecd00

HansMuller approved these changes Aug 21, 2019

View reviewed changes

Additional clarification to API docs

3bbcc30

shihaohong merged commit 3b74104 into flutter:master Aug 22, 2019

shihaohong mentioned this pull request Sep 5, 2019

Android 10 / Android Q Issues and Features #39858

Closed

5 tasks

HansMuller mentioned this pull request Sep 6, 2019

Planned Material Library Features Jul-Sept 2019 #35088

Closed

15 tasks

shihaohong deleted the android-q-system-gesture-navigation branch September 10, 2019 16:20

github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2021

Conversation

shihaohong commented Aug 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

shihaohong commented Aug 7, 2019

Uh oh!

shihaohong commented Aug 14, 2019

Uh oh!

goderbauer 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!

HansMuller Aug 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

shihaohong Aug 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

shihaohong Aug 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

shihaohong Aug 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

HansMuller 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!

Reviewers

Assignees

Labels

shihaohong commented Aug 1, 2019 •

edited

Loading

HansMuller Aug 21, 2019 •

edited

Loading

shihaohong Aug 22, 2019 •

edited

Loading

shihaohong Aug 21, 2019 •

edited

Loading

shihaohong Aug 21, 2019 •

edited

Loading