PageTransitionsTheme, new MountainView page transition #21715

HansMuller · 2018-09-12T00:40:21Z

MaterialPageRoute transitions are now defined by the Theme. Added (optional) support for Android P style page transitions.

The default MaterialPageRoute transitions have not changed: the horizontal CuperitinoPageTransition is still used when the target platform is iOS and the fading upwards transition is used for all other target platforms.

Added a ThemeData.pageTransitionsTheme which provides the default implementation of MaterialPageRoute.buildTransitions per target platform. Subclasses of PageTransitionsBuilder define buildTransitions methods.

Three PageTransitionsBuilder subclasses are provided: FadeUpwardsTransitionsBuilder, CupertinoPageTransitionsBuilder, OpenUpwardsPageTransitionsBuilder.

By default PageTransitionsTheme only includes FadeUpwardsPageTransitionsBuilder, and CupertinoPageTransitionsBuilder.

To use the new Android P style page transition when the target platform is Android, include OpenUpwardsPageTransitionsBuilder in the Theme's pageTransitionsTheme. For example:

new MaterialApp(
  theme: new ThemeData(
    pageTransitionsTheme: const PageTransitionsTheme(
      builders: <TargetPlatform, PageTransitionsBuilder>{
        TargetPlatform.iOS: CupertinoPageTransitionsBuilder(),
        TargetPlatform.android: OpenUpwardsPageTransitionsBuilder(),
      },
    ),
  ),
  ...
)

Two of the CupertinoPageRoute methods that had been used to enable building a MaterialPageRoute that used a "host" CupertinoPageRoute's buildTransitions() method have been removed or changed:

PageRoute<T> get hostRoute;  // also removed the constructor parameter
bool get popGestureInProgress;

The last method was replaced by a static method that reports the same value for any route that's using a Cupertino transition:

  static bool isPopGestureInProgress(PageRoute route)

These are backwards incompatible changes. However it seems unlikely that any existing apps depended on them.

Hixie · 2018-09-13T20:26:34Z

packages/flutter/lib/src/material/page_transitions_theme.dart

please run flutter analyze --dartdocs to catch places where we need to add docs.

Hixie · 2018-09-13T21:45:27Z

packages/flutter/lib/src/cupertino/route.dart

This is technically a breaking change so we should definitely mention it on flutter-dev, but I find it hard to imagine that it's going to break any actual apps since it doesn't really do anything.

I sent a warning about this change to flutter-dev, subject "Proposed changes to the CupertinoPageRoute class", on 9/10. Will definitely send a definitive message when this PR lands.

Great, thanks.

Hixie · 2018-09-13T23:23:10Z

packages/flutter/lib/src/cupertino/route.dart

nit: s/ - /, /

Hixie · 2018-09-13T23:24:07Z

packages/flutter/lib/src/cupertino/route.dart

nit: (not new, but since you're here) return after else

I assumed that what's needed here is:

assert(false); return null;

At the end of the method. The analyzer reports that as dead code. What did you want here?

I just meant that you could remove the else { part and just have that be at the top level of the method.

Hixie · 2018-09-13T23:25:21Z

packages/flutter/lib/src/cupertino/route.dart

docs need updating a bit

Hixie · 2018-09-13T23:37:53Z

packages/flutter/lib/src/material/page_transitions_theme.dart

can you pass transformHitTests: false to this widget and remove the TODO while you're here?

I undid this change. It cause a few tests to fail and in the interest of limiting the scope of this PR, I've put the TODO back for now.

Hixie · 2018-09-13T23:38:35Z

packages/flutter/lib/src/material/page_transitions_theme.dart

do we need the clip?

Yes. The new page is pinned to the bottom and the clip exposes more and more of it.

If it's pinned to the bottom of the screen then we don't need to clip it, since it's overflowing the screen where the physical device will handle clipping for us, right?

Other than a tiny translation, the new page appears exactly where it will end up (it doesn't overflow). The clip is like a vertical curtain that's pulled upwards: so the bottom of the page is exposed first.

Hixie · 2018-09-13T23:39:28Z

packages/flutter/lib/src/material/page_transitions_theme.dart

I would call this (and the previous one) something more descriptive.

Hixie · 2018-09-13T23:40:08Z

packages/flutter/lib/src/material/page_transitions_theme.dart

we should avoid using underscores for argument names, it just makes people confused when reading the code

Sorry about. Plus now I'm using the parameter.

Hixie · 2018-09-13T23:40:30Z

packages/flutter/lib/src/material/page_transitions_theme.dart

you can make this a child of the outer AnimatedBuilder to avoid rebuilding the inner one each time

Good point! Done.

Hixie · 2018-09-13T23:43:12Z

packages/flutter/lib/src/material/page_transitions_theme.dart

shouldn't this be in the cupertino library?

oh, i see, this is sort of specific to material...

Hixie · 2018-09-13T23:44:52Z

packages/flutter/lib/src/material/page_transitions_theme.dart

this seems overly general when the reality is we just care about iOS vs anything else.

maybe we care about more than that though? I don't really understand why the PageTransitionsBuilders have explicit platforms, and why the general one is different than the mountain view one.

This PR enables one to add an Android-specific transition, the MountainViewPageTransition. I've enabled configuring the default transition for any platform, which is indeed over achievement, however I think it's a useful point for customization.

The default generic transition is identical to the one we use on Android/Fuschia today. It's no longer part of the Material Spec and it's very different from the default transition on the latest Android (P).

Hixie · 2018-09-13T23:45:53Z

packages/flutter/lib/src/material/page_transitions_theme.dart

is there much benefit to having the _GenericPageTransition widget rather than just inlining its build method here?

This way you can substitute your own generic (not target platform specific) page transition.

I don't see how it helps with this? The widgets are private, so the effect seems to be identical, from a black-box perspective, as having the widgets inlined.

Hixie · 2018-09-13T23:46:21Z

packages/flutter/lib/src/material/theme_data.dart

boy is this out of hand

Hixie · 2018-09-13T23:46:59Z

packages/flutter/lib/src/material/theme_data.dart

please make sure these are in the exact same order as in operator == and in the constructor and in the order of fields in the class

and in the lerps

Fixed up the order of everything and added some missing (yikes) arguments to hashValues/==.

Hixie · 2018-09-13T23:48:26Z

packages/flutter/lib/src/material/theme_data.dart

Not quite sure what "per [TargetPlatform]" means here. Maybe elaborate a bit for these docs?

Hixie · 2018-09-19T18:39:39Z

packages/flutter/lib/src/cupertino/route.dart

can we keep this one around for back-compat? It can be implemented just like popGestureEnabled, using the statics and this.

Hixie · 2018-09-19T18:44:55Z

I have to be honest the whole thing with the transitions having platforms feels very strange to me. I don't see why the opening up transition has any affinity to Android, and why the sliding one doesn't. I understand why the Cupertino one does, but only insofar as we have to use that one to enable the back gesture. Why can't the Theme declare what the right transition is based on the platform, rather than have the transitions think about the issue? (Notwithstanding that once you've picked a transition, it shouldn't change until it's over, even if you change platform, so that we don't crash when switching out of iOS mode with an active back gesture.)

HansMuller · 2018-09-19T23:25:28Z

PageTransitionsBuilders no longer have a platform. The PageTransitionsTheme now has a TargetPlatform => PageTransitionsBuilder map.

Hixie · 2018-09-24T19:37:30Z

packages/flutter/lib/src/cupertino/route.dart

this one should probably transform if it's being driven by the user and not otherwise... :-/

Hixie · 2018-09-24T19:38:42Z

packages/flutter/lib/src/material/page_transitions_theme.dart

still mentions platform

Hixie · 2018-09-24T19:38:51Z

packages/flutter/lib/src/material/page_transitions_theme.dart

[] around the class name

Hixie · 2018-09-24T19:38:55Z

packages/flutter/lib/src/material/page_transitions_theme.dart

Hixie · 2018-09-24T19:39:00Z

packages/flutter/lib/src/material/page_transitions_theme.dart

Hixie · 2018-09-24T19:39:34Z

packages/flutter/lib/src/material/page_transitions_theme.dart

this paragraph is obsolete

Hixie · 2018-09-24T19:41:15Z

packages/flutter/lib/src/material/page_transitions_theme.dart

Should this use the iOS platform to look it up in the map, or should it forcibly use the cupertino transition?

I think it makes sense to continue using whatever transition was defined for iOS. That's CupertinoTransition by default of course.

Hixie · 2018-09-24T19:44:11Z

packages/flutter/lib/src/material/page_transitions_theme.dart

maybe ?? const FadeUpwardsPageTransitionsBuilder() ?

This method is just for the operator == definition. I added an explanatory comment.

Hixie · 2018-09-24T19:44:35Z

packages/flutter/lib/src/material/page_transitions_theme.dart

might be good to precache this on creation, since i imagine (hope?) that this class will be compared more often than created...

The operator == method only uses this value when the lists of builders aren't identical. Hopefully that's a rarity. If not I'll add some caching.

Hixie · 2018-09-24T19:45:24Z

packages/flutter/lib/src/material/page_transitions_theme.dart

you could just hand in _defaultBuilders directly instead of the dance here with defaultTheme, no?

Hixie · 2018-09-24T19:45:39Z

packages/flutter/lib/src/material/page_transitions_theme.dart

i think this can be made const

Yes, good point.

Hixie · 2018-09-24T19:45:52Z

xster

LGTM modulo the 'Open' name. I wish we can call it something more visually descriptive.

xster · 2018-09-24T22:46:27Z

packages/flutter/lib/src/cupertino/route.dart

Needs a small tweak. As-is, it reads a bit as if this class itself doesn't use this.

xster · 2018-09-24T22:48:31Z

packages/flutter/lib/src/material/page_transitions_theme.dart

For future readers, describe which Android versions this corresponds too.

xster · 2018-09-24T22:50:18Z

packages/flutter/lib/src/material/page_transitions_theme.dart

Describe the motion briefly?

Since this is // internal documentation, I included the motion descriptions with the Tweens that define them.

The corresponding (public) PageTransitionBuilder does explain the motion in its class dartdoc.

xster · 2018-09-24T22:50:51Z

packages/flutter/lib/src/material/page_transitions_theme.dart

'Open' isn't very visually descriptive. Perhaps 'Parallax' or 'Reveal'?

I like calling this transition OpenUpwards. The new page doesn't really move (not much), but we "open" a view of it that expands upwards.

xster · 2018-09-24T22:54:38Z

packages/flutter/lib/src/material/page_transitions_theme.dart

Chatted a bit offline. Conceptually, this sounds a bit suspicious

I've reversed the direction, which undid a last-minute and ill considered change.

xster · 2018-09-24T22:55:28Z

packages/flutter/lib/src/material/page_transitions_theme.dart

xster · 2018-09-24T22:57:09Z

packages/flutter/lib/src/material/page_transitions_theme.dart

No change needed, though I'm a bit surprised we don't have a FractionalClip or something.

xster · 2018-09-24T22:57:52Z

packages/flutter/lib/src/material/page_transitions_theme.dart

Maybe just create this once since you use it 3 times

xster · 2018-09-24T22:59:52Z

packages/flutter/lib/src/material/page_transitions_theme.dart

Add a small commentary for when users should choose to use this

At the moment I don't have anything to add. It's the default, same as it ever was.

It was based on video content from the original Material spec and it's common in Material Android apps before version P.

xster · 2018-09-24T23:01:51Z

packages/flutter/lib/src/material/page_transitions_theme.dart

Open still doesn't feel descriptive. I'm sure there are prior-art from things like PowerPoint or Adobe Premier that we can re-use.

So PowerPoint calls it 'Wipe' or 'Cover' and Apple Keynote calls it 'Reveal'. I think any of them are an improvement on 'Open'

I think they're all OK, and I'm happy with "open".

googlebot added the cla: yes label Sep 12, 2018

HansMuller force-pushed the page_transition_theme branch from 74af122 to 2696f0f Compare September 12, 2018 16:13

Hixie reviewed Sep 13, 2018

View reviewed changes

packages/flutter/lib/src/material/theme_data.dart Outdated

Copy link

Contributor

Hixie Sep 13, 2018

Choose a reason for hiding this comment

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

boy is this out of hand

Hixie reviewed Sep 13, 2018

View reviewed changes

HansMuller force-pushed the page_transition_theme branch from 8f7f512 to a7adfaf Compare September 14, 2018 18:18

Hixie reviewed Sep 19, 2018

View reviewed changes

HansMuller force-pushed the page_transition_theme branch from a1cce5c to c8f8105 Compare September 19, 2018 23:23

HansMuller force-pushed the page_transition_theme branch from c8f8105 to 7b2f3ae Compare September 24, 2018 17:03

Hixie reviewed Sep 24, 2018

View reviewed changes

packages/flutter/lib/src/material/page_transitions_theme.dart Outdated

Copy link

Contributor

Hixie Sep 24, 2018

Choose a reason for hiding this comment

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

still mentions platform

Hixie reviewed Sep 24, 2018

View reviewed changes

packages/flutter/lib/src/material/page_transitions_theme.dart Outdated

Copy link

Contributor

Hixie Sep 24, 2018

Choose a reason for hiding this comment

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

[] around the class name

Hixie reviewed Sep 24, 2018

View reviewed changes

packages/flutter/lib/src/material/page_transitions_theme.dart Outdated

Copy link

Contributor

Hixie Sep 24, 2018

Choose a reason for hiding this comment

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

ditto

Hixie reviewed Sep 24, 2018

View reviewed changes

packages/flutter/lib/src/material/page_transitions_theme.dart Outdated

Copy link

Contributor

Hixie Sep 24, 2018

Choose a reason for hiding this comment

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

ditto

Hixie reviewed Sep 24, 2018

View reviewed changes

packages/flutter/lib/src/material/page_transitions_theme.dart Outdated

Copy link

Contributor

Hixie Sep 24, 2018

Choose a reason for hiding this comment

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

this paragraph is obsolete

Hixie reviewed Sep 24, 2018

View reviewed changes

HansMuller mentioned this pull request Sep 24, 2018

Refactor of cupertino/material/widgets app #22161

Merged

HansMuller force-pushed the page_transition_theme branch 2 times, most recently from dc5e37c to 7b9152f Compare September 24, 2018 23:03

xster approved these changes Sep 24, 2018

View reviewed changes

HansMuller added 11 commits September 24, 2018 17:31

PageTransitionsTheme, new MountainView page transition

628c177

removed unnecessary_new lint info

d6aef18

Updated per review feedback

0b4a82f

update per review feedback

f2013bf

Renaming per review feedback

97a927a

PageTransitionsBuilders no longer have-a platform

714b7da

update

f02ea20

updated per review feedback

a1f795d

fix secondary animation direction

1a31c3e

updated per review feedback

53d6b1f

updated

0d98dcc

HansMuller force-pushed the page_transition_theme branch from a766aeb to 0d98dcc Compare September 25, 2018 00:36

HansMuller merged commit 582f35d into flutter:master Sep 25, 2018

HansMuller deleted the page_transition_theme branch September 25, 2018 17:56

noelmansour mentioned this pull request Oct 5, 2018

Old screen transitions are used on Android API 28 #21191

Closed

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

PageTransitionsTheme, new MountainView page transition #21715

PageTransitionsTheme, new MountainView page transition #21715

Uh oh!

Conversation

HansMuller commented Sep 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HansMuller Sep 14, 2018 • 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!

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!

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!

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!

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

HansMuller commented Sep 12, 2018 •

edited

Loading

HansMuller Sep 14, 2018 •

edited

Loading