Foldable support & TwoPane Widget by andreidiaconu · Pull Request #77156 · flutter/flutter

andreidiaconu · 2021-03-03T16:43:20Z

Adds support for foldable devices and display cutouts, in the form of Display Features exposed by MediaQuery.
Adds TwoPane, a widget which helps with layouts on dual-screen devices and scaling a two pane design on larger screens.
Adds HingeAvoidingModalWrapper, a widget can be used at the root of a Popup route in order to make the contents avoid the hinge area. It determines the safe areas of the display and uses one of them to render its child.
Makes dialogs aware of the hinge, so they don't overlap.

This is based on the Engine support for Display Features: flutter/engine#24756

A display feature is an area of the display that is obstructed by a hardware feature. For now, this works only on Android. There are three types of display features:

Hinge - a physical separator between the two displays of the device.
Fold - You can view the fold like a hinge that has zero width. It tells us where the flexible display has a crease.
Cutout - A cutout sits at the edge of the display and usually houses camera systems.

Here is a preview of what this PR enables, on a dual-screen emulator:

Issues that will be closed by this PR:

[android] support android 12L #49411

Work in progress:

Tests are still in progress
The engine PR needs to be closed first: Display Features support (Foldable and Cutout) engine#24756

…_support

flutter-dashboard · 2021-03-03T16:43:22Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

goderbauer · 2021-03-16T18:57:49Z

Thanks for the PR. Unfortunately, this PR is in many aspects not following our style guide (see also https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo). Most of the issues are also pointed out by the analyzer (flutter analyze --flutter-repo). There's also commented out code in the PR making it unclear whether this is just a draft or if it's actually ready for review. Can you please clean this up to make it easier to actually review this? Thanks!

andreidiaconu · 2021-03-16T19:06:09Z

@goderbauer Yes. Will ping you when this is higher quality and also has tests.

goderbauer · 2021-03-16T19:26:45Z

OK, marking this as draft for now.

flutter-dashboard · 2021-03-16T19:31:30Z

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

goderbauer · 2021-03-17T16:12:46Z

Some overall feedback from scanning through the code: It would be good to keep this a little more generic. The Dialogs and Pop-ups for example need to avoid all kinds of display obstructions and the API shouldn't assume that these obstructions divide the rendering space into two "screens" (not all obstructions will do). So, having a screen argument doesn't seem ideal. As an idea, maybe we could have an anchorPoint argument instead that specifies if the dialog cannot be rendered at its natural point (e.g. due to obstructions) it is pulled towards that point to stay clear of those obstructions. The point the user touched on screen to trigger the dialog could be used as a natural anchor point for most use cases, presumably. If no anchor point is provided, we'd use a heuristic to avoid obstructions (e.g. prefer higher over lower, prefer the leading edge over the trailing edge, etc).

Also, the TwoPane seems pretty specific to certain phone hardware and its also pretty independent of the framework. That widget may be best distributed as part of an external package.

andreidiaconu · 2021-03-17T19:55:23Z

I see your point on dialogs and will give the anchorPoint a try. I think it is a good idea, but wonder if it complicates the api more than needed. Renaming screen to safeArea would be less specific to dual screen form factors. TwoPane indeed helps with dual screen devices a lot, but is meant to easily scale up the same layout to tablets, desktop and web. This is why I don’t see it as a dual screen specific layout, but rather as a basic building block that happens to work well on that form factor.

…

On Wed, 17 Mar 2021 at 18:13, Michael Goderbauer ***@***.***> wrote: Some overall feedback from scanning through the code: It would be good to keep this a little more generic. The Dialogs and Pop-ups for example need to avoid all kinds of display obstructions and the API shouldn't assume that these obstructions divide the rendering space into two "screens" (not all obstructions will do). So, having a screen argument doesn't seem ideal. As an idea, maybe we could have an anchorPoint argument instead that specifies if the dialog cannot be rendered at its natural point (e.g. due to obstructions) it is pulled towards that point to stay clear of those obstructions. The point the user touched on screen to trigger the dialog could be used as a natural anchor point for most use cases, presumably. If no anchor point is provided, we'd use a heuristic to avoid obstructions (e.g. prefer higher over lower, prefer the leading edge over the trailing edge, etc). Also, the TwoPane seems pretty specific to certain phone hardware and its also pretty independent of the framework. That widget may be best distributed as part of an external package. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#77156 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAKWJPU2FBV34NKSSSF72VTTEDIJLANCNFSM4YRSE4EA> .

andreidiaconu · 2021-03-22T17:17:11Z

@goderbauer The code is now in line with the style guide and gives no messages during analyze. Made the dialogs use anchorPoint and fallback on Directionality. I'm turning my attention to tests. Since we want all checks to pass before we consider this Ready for review, this means that flutter/engine#24756 needs to be merged first - I will update you when we make progress on that.

For now I just wanted to let you know that your concerns were addressed, in case you want to give this another look and provide any other feedback.

…port # Conflicts: # packages/flutter/lib/src/material/bottom_sheet.dart # packages/flutter/lib/src/material/date_picker.dart # packages/flutter/lib/src/material/popup_menu.dart # packages/flutter/lib/src/widgets/routes.dart

…port # Conflicts: # packages/flutter/lib/src/widgets/media_query.dart # packages/flutter/test/widgets/media_query_test.dart

andreidiaconu · 2021-08-23T09:14:36Z

@goderbauer I would like to offer an update on this PR:

The Engine PR that this depends on is now out of WIP status. It was marked as WIP while we waited for its main dependency, androidx.window, to be promoted to beta.
This PR now has tests for all the components it touches and the documentation is better.

The current status for this PR is that we are waiting for the Engine PR to be merged, but the code is ready for review, in case we want to speed things along in the meantime. I will gladly address any feedback I get, as I already did up to this point.

blasten · 2021-08-26T21:11:31Z

PTAL @goderbauer

…port # Conflicts: # packages/flutter/test/material/bottom_sheet_test.dart

goderbauer · 2021-09-07T19:09:11Z

I believe this PR would be easier to review, land, and (if necessary) revert if it were split up into multiple PRs, one per feature listed in the bullet list in the PR's description.

andreidiaconu · 2021-09-07T19:12:33Z

Ok, I will split this into multiple PRs.

12people · 2021-10-31T21:37:30Z

@andreidiaconu Any updates on this?

andreidiaconu · 2021-11-01T07:54:27Z

@andreidiaconu Any updates on this?

Yes, the Engine changes are now able to make progress #89511 and once those are merged, focus moves to this. Also, I am breaking ths PR into smaller, more focused ones this week.

…port # Conflicts: # packages/flutter/lib/src/material/time_picker.dart # packages/flutter/test/material/bottom_sheet_test.dart

andreidiaconu · 2021-11-02T12:29:25Z

Here are the PRs that were split from this:

Edit: And they can be reviewed in that order. The first one would be #92906 since all the other depend on it.

Everything still depends on getting the Engine support merged:

Engine support: Reland Display Features support (Foldable and Cutout) engine#29447
Framework integration of the engine support: Reland engine display features #89511

@blasten @goderbauer I am closing this PR as it is now replaced by 4 other smaller PRs.

andreidiaconu added 4 commits February 3, 2021 18:39

MediaQuery, Routes and TwoPane changes

47254b4

Better documentation

b90b80d

Merge commit 'dff42b8ee143460b322e50cada738682ca7fbee1' into foldable…

8de5789

…_support

Screen parameter for hinge aware routes

f43b34f

flutter-dashboard bot added f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Mar 3, 2021

google-cla bot added the cla: yes label Mar 3, 2021

goderbauer self-requested a review March 4, 2021 16:58

goderbauer marked this pull request as draft March 16, 2021 19:26

Merge remote-tracking branch 'upstream/master' into foldable_support

1ba3482

andreidiaconu added 3 commits March 22, 2021 12:01

Fixed analyze issues

28ff1d0

Fixed whitespace and copyright analyze issues

df2d03f

Offset anchorPoint parameter instead of screen number for dialogs.

04d166b

andreidiaconu added 2 commits July 5, 2021 18:02

Add padding to TwoPane

9edc6e7

andreidiaconu force-pushed the foldable_support branch from 3b8080a to 9587c46 Compare July 7, 2021 13:09

andreidiaconu added 5 commits July 19, 2021 17:19

Test display feature changes

0ce75da

Fix license header for bottom_sheet_test.dart

57065f4

Fix TwoPane Directionality issues

a1ca27a

Merge branch 'master' of github.com:flutter/flutter into foldable_sup…

a5274d6

…port # Conflicts: # packages/flutter/lib/src/widgets/media_query.dart # packages/flutter/test/widgets/media_query_test.dart

Improve MediaQueryData equality for display features

a3e30c6

Merge branch 'master' of github.com:flutter/flutter into foldable_sup…

33823d7

…port # Conflicts: # packages/flutter/test/material/bottom_sheet_test.dart

andreidiaconu added 3 commits November 2, 2021 11:25

Merge branch 'master' of github.com:flutter/flutter into foldable_sup…

a59fd90

…port # Conflicts: # packages/flutter/lib/src/material/time_picker.dart # packages/flutter/test/material/bottom_sheet_test.dart

Separate DisplayFeatureSubScreen into its own file

fcbcf60

Fix analyze issues

f1828f5

This was referenced Nov 2, 2021

Add display features to MediaQuery #92906

Merged

Add DisplayFeatureSubScreen widget #92907

Merged

Add Foldable support for modal routes #92909

Merged

Add TwoPane widget #92910

Closed

andreidiaconu closed this Nov 2, 2021

andreidiaconu mentioned this pull request Feb 23, 2022

Make popup menus avoid display features #98981

Merged

8 tasks

Conversation

andreidiaconu commented Mar 3, 2021

Work in progress:

Uh oh!

flutter-dashboard bot commented Mar 3, 2021

Uh oh!

goderbauer commented Mar 16, 2021

Uh oh!

andreidiaconu commented Mar 16, 2021

Uh oh!

goderbauer commented Mar 16, 2021

Uh oh!

flutter-dashboard bot commented Mar 16, 2021

Uh oh!

goderbauer commented Mar 17, 2021

Uh oh!

andreidiaconu commented Mar 17, 2021 via email

Uh oh!

andreidiaconu commented Mar 22, 2021

Uh oh!

andreidiaconu commented Aug 23, 2021

Uh oh!

blasten commented Aug 26, 2021

Uh oh!

goderbauer commented Sep 7, 2021

Uh oh!

andreidiaconu commented Sep 7, 2021

Uh oh!

12people commented Oct 31, 2021

Uh oh!

andreidiaconu commented Nov 1, 2021

Uh oh!

andreidiaconu commented Nov 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andreidiaconu commented Nov 2, 2021 •

edited

Loading