-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Foldable support #677
Foldable support #677
Conversation
…_support # Conflicts: # lib/data/demos.dart # lib/routes.dart
# Conflicts: # lib/data/demos.dart # lib/layout/adaptive.dart # lib/pages/home.dart # lib/pages/splash.dart # lib/routes.dart
Benchmark and golden tests are failing but I can't figure out why. Marking this as ready for review while I try to understand the error in the benchmark test. LE: Benchmark seems to pass now. |
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.
LGTM overall.
I wonder if it would be beneficial to s/dual/hinged/g everywhere. I feel like "hinged screen" is more common than "dual screen"
This works on all foldable devices, although the split in the middle of the layout happens on continuous screens only when the hinge is not flat. So for a Samsung Fold for example, the layout is not considered "foldable" if the screen is flat, but it then converts to a foldable layout when you start bringing the screens together (using it like a book). I will go over the texts and naming to make sure this no longer points to "dual-screen" and points to "foldable" instead, as a general term. |
@guidezpl Made most of the changes you requested.
|
@guidezpl Updated to latest master, with super constructor parameters. The only failing test is the Golden one. I could update the golden files, but I'm not sure if this requires a linux machine or if I can do it on a Windows or macOS myself. |
Awesome, thanks! The updated goldens are available at https://github.com/flutter/gallery/actions/runs/2333115719. I have just a few small nits around language |
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.
I think the new dialog option isn't best placed in the gallery, since it is primarily used on non-foldable devices, web especially. I would remove it and instead suggest adding the code as code snippets in the package API docs.
@guidezpl Not sure why goldens are still failing. It tries to create a new artifact but the failures folder does not exist. Seems to fail from unusual reasons. Do you know if this is happening on other PRs or branches? (If it is not related to the changes I made, a separate PR to address goldens failing might be a better idea than to fix here) LE: Nevermind, I found the problem and it is seems to be related to changes I made. |
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.
The goldens need to be updated for new description, I think, otherwise LGTM! Happy to merge this after the goldens issue is figured out
Thank you! |
Adds support for foldable devices (support newly added to Flutter), as shown in the following images. It also adds a new sample, related to foldable development. It showcases TwoPane, the same widget used in making the Gallery app foldable-aware.
Here is a screenshot of the TwoPane sample:
Pre-launch Checklist
///
).