Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

There should be no default page transitions for desktop and web platforms. #99052

Open
darrenaustin opened this issue Feb 24, 2022 · 16 comments
Open
Labels
a: animation Animation APIs a: desktop Running on desktop c: new feature Nothing broken; request for a new capability c: proposal A detailed proposal for a change to Flutter customer: google Various Google teams f: material design flutter/packages/flutter/material repository. f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels. P3 Issues that are less important to the Flutter project team-desktop Owned by Desktop platforms team triaged-desktop Triaged by Desktop platforms team

Comments

@darrenaustin
Copy link
Contributor

As page transitions are not common on desktop and web applications, the default transitions on these platforms should be removed.

An attempt at this was tried with #82596, but it had an issue with the delay was still occurring even though the transition gone.

@darrenaustin darrenaustin added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Feb 24, 2022
@maheshmnj maheshmnj added a: animation Animation APIs a: desktop Running on desktop f: routes Navigator, Router, and related APIs. c: proposal A detailed proposal for a change to Flutter c: new feature Nothing broken; request for a new capability labels Mar 4, 2022
@johnpryan
Copy link
Contributor

johnpryan commented Mar 4, 2022

Right now users can use a custom Page object that disables the transition, but requires some custom code to switch based on the platform. Also, there needs to be a 1ms delay when this is used with go_router.

Instead, does it make sense to define a new PageTransitionsBuilder?

class NoAnimationPageTransitionsBuilder extends PageTransitionsBuilder {
  const NoAnimationPageTransitionsBuilder();

  @override
  Widget buildTransitions<T>(
      PageRoute<T> route,
      BuildContext context,
      Animation<double> animation,
      Animation<double> secondaryAnimation,
      Widget child,
      ) {
    return child;
  }
}

And then use that as the default for linux, macOS, and Windows in the PageTransitionsTheme class?

  static const Map<TargetPlatform, PageTransitionsBuilder> _defaultBuilders = <TargetPlatform, PageTransitionsBuilder>{
    TargetPlatform.android: FadeUpwardsPageTransitionsBuilder(),
    TargetPlatform.iOS: CupertinoPageTransitionsBuilder(),
    TargetPlatform.linux: FadeUpwardsPageTransitionsBuilder(), // Should be NoAnimationPageTransitionsBuilder?
    TargetPlatform.macOS: CupertinoPageTransitionsBuilder(), // Should be NoAnimationPageTransitionsBuilder?
    TargetPlatform.windows: FadeUpwardsPageTransitionsBuilder(), // Should be NoAnimationPageTransitionsBuilder?
  };

Am I understanding correctly that the only other thing that's needed is a way to customize the transition duration based on the platform?

@gspencergoog gspencergoog added the P3 Issues that are less important to the Flutter project label Mar 10, 2022
@nt4f04uNd
Copy link
Member

Also, #95764 seems related

@braj065
Copy link

braj065 commented Mar 14, 2022

Does this also looks related? - #41564

@willlockwood
Copy link
Contributor

willlockwood commented May 20, 2022

Also, #95764 seems related

Yes, that was my initial thought as well. I think likely the same fix would solve both of these. Basically, the solution would be that the PageTransitionsTheme should in some way be able to specify durations (as described in a very basic sense on that issue).

If no one is currently working this, I can try to figure that out 👍🏻

Does this also looks related? - #41564

Yup I think the same fix can probably encapsulate this one as well.

@jacobsimionato
Copy link
Contributor

jacobsimionato commented Aug 2, 2022

I think this could be considered a bug with higher priority than P5 rather than a feature request, because clients are having to work around this issue currently to meet user expectations. Also, we're looking at changing the default behavior which could be surprising / disruptive to some apps, and so the sooner we do it, the less apps will be disrupted.

See internal Google document https://docs.google.com/document/d/1MPf4N81YWppEgEN8dqwwhTwTBliV_T0jCbbdaSdefaQ/edit#bookmark=id.qsc6q9wh6nxu for more context.

@tmpfs
Copy link

tmpfs commented Feb 26, 2023

I agree with @jacobsimionato this should be higher priority, for those coming across this issue here is a workaround:

class NoPageTransitionsBuilder extends PageTransitionsBuilder {
  const NoPageTransitionsBuilder();

  @override
  Widget buildTransitions<T>(
    PageRoute<T>? route,
    BuildContext? context,
    Animation<double> animation,
    Animation<double>? secondaryAnimation,
    Widget child,
  ) {
    return child;
  }
}

And then in ThemeData configure the pageTransitionsTheme:

    pageTransitionsTheme: const PageTransitionsTheme(
      builders: <TargetPlatform, PageTransitionsBuilder>{
        TargetPlatform.android: FadeUpwardsPageTransitionsBuilder(),
        TargetPlatform.iOS: CupertinoPageTransitionsBuilder(),
        TargetPlatform.linux: NoPageTransitionsBuilder(),
        TargetPlatform.macOS: NoPageTransitionsBuilder(),
        TargetPlatform.windows: NoPageTransitionsBuilder(),
      },
    ),

@rydmike
Copy link
Contributor

rydmike commented Feb 26, 2023

@jacobsimionato I'm getting an access denied error when trying to read the internal Google document you linked.

https://docs.google.com/document/d/1MPf4N81YWppEgEN8dqwwhTwTBliV_T0jCbbdaSdefaQ/edit#bookmark=id.qsc6q9wh6nxu

If possible, can you share with at least read access?

Looks very interesting and relevant to something I'm working on at the moment.

@jacobsimionato
Copy link
Contributor

Hi rydmike, I'm sorry I can't share that internal doc. My apologies for linking to an inaccessible doc here! It doesn't say much more than what is already on this issue though - I think its important that Flutter has sensible defaults for each platform, including the newer ones, and that these are based on end user expectations.

@hangyujin hangyujin removed their assignment May 2, 2023
@flutter-triage-bot flutter-triage-bot bot added multiteam-retriage-candidate team-desktop Owned by Desktop platforms team triaged-desktop Triaged by Desktop platforms team labels Jul 7, 2023
@benhawkinsicon
Copy link

benhawkinsicon commented Aug 4, 2023

Am I correct in saying there is no way to change default page transition durations?

The workaround @tmpfs posted doesn't remove the secondaryAnimation duration, so it fixes navigating to a page but not back. The only way to make web routing behave correctly is to wrap every individual route in a NoTransitionPage?

If the only workaround requires extra code on every route, I would agree it seems worth fixing

@benhawkinsicon
Copy link

benhawkinsicon commented Aug 4, 2023

Example:

Using @tmpfs example yields a back delay:

with_delay.mov

Only possible to remove by wrapping in NoTransitionPage:

without_delay.mov

So we actually shouldn't even touch PageTransitionsTheme as NoTransitionPage on every route makes it redundant.

@tmpfs
Copy link

tmpfs commented Aug 5, 2023

@benhawkinsicon , yes that is annoying, there are other issues about the delay problem:

#27777
#88221

I really wish this was fixed properly in the Flutter framework but for now all I have is this workaround.

@danwild
Copy link

danwild commented Sep 17, 2023

+1

I'm 2 days into building my first app in flutter and I was immediately confused by why my large web pages routes were awkwardly animating.

To make things worse (i.e. more complex..), I want to make my app adaptive.

In the adaptive paradigm, I would argue that:

  1. animations for a web app running on a LARGE screen (i.e. desktop browser) make no sense; but
  2. animations for a web app on a SMALL screen make perfect sense.. (e.g. a Drawer widget animating the slide open/close on a mobile browser).

The above workaround (thanks @tmpfs ! I'll use this pattern until something better presents) seems to be platform specific (i.e. blanket case for all web screens) and doesn't cover my use-case, which I suspect would be a common one.

Following 👀

@tmpfs
Copy link

tmpfs commented Oct 26, 2023

@darrenaustin is there an idea of when this might land please? I find this delay on back navigation personally very frustrating as I navigate the app (300ms is very noticeable) and I am sure my users will find the delay perplexing.

If there is something I can do to help move this forward please let me know.

@eEQK
Copy link
Contributor

eEQK commented Apr 7, 2024

This can be worked around by specifying PageRoute.reverseTransitionDuration

go_router example, but CustomTransitionPage is from SDK:

  @override
  Page<void> buildPage(BuildContext context, GoRouterState state) =>
      CustomTransitionPage(
        child: const FooScreen(),
        key: state.pageKey,
        transitionDuration: Duration.zero,
        reverseTransitionDuration: Duration.zero,
        transitionsBuilder: (_, __, ___, child) => child,
      );

more practical example:

@immutable
class HomeRoute extends GoRouteData {
  const HomeRoute();

  @override
  Page<void> buildPage(BuildContext context, GoRouterState state) =>
      _NoTransition(state, child: const HomeScreen());
}


class _NoTransition extends CustomTransitionPage {
  _NoTransition(GoRouterState state, {required super.child})
      : super(
          key: state.pageKey,
          transitionsBuilder: (_, __, ___, child) => child,
          reverseTransitionDuration: Duration.zero,
          transitionDuration: Duration.zero,
        );
}

@tmpfs
Copy link

tmpfs commented Apr 8, 2024

@eEQK , this isn't working for me.

I added this to a pageBuilder and I still see the delay:

class _NoTransition extends CustomTransitionPage {
  _NoTransition(GoRouterState state, {required super.child})
      : super(
          key: state.pageKey,
          transitionsBuilder: (_, __, ___, child) => child,
          reverseTransitionDuration: Duration.zero,
          transitionDuration: Duration.zero,
        );
}

// Standard page builder.
Page<dynamic> pageBuilder(
    BuildContext context, GoRouterState state, Widget child) {
  
  if (isDesktop()) {
    return _NoTransition(state, child: child);
  }

  return MaterialPage(child: child);
}

Then I assign the pageBuilder to the route and still the standard 300ms delay. Any ideas?

Edit: apparently assigning to the pageBuilder for a ShellRoute does not work, in the end I assigned pageBuilder to each individual route and then just called this function in each route:

Page<dynamic> noTransitionPageBuilder(Widget child) {
  if (isDesktop()) {
    return NoTransitionPage(child: child);
  }
  return MaterialPage(child: child);
}

The _NoTransition class is not needed, just use NoTransitionPage from the go_router package.

@eEQK
Copy link
Contributor

eEQK commented Apr 8, 2024

Not sure how I can help you @tmpfs since I created a minimal example and it works:
https://dartpad.dev/?id=9c8f887f324018bceb11c7c7406c0ba3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: animation Animation APIs a: desktop Running on desktop c: new feature Nothing broken; request for a new capability c: proposal A detailed proposal for a change to Flutter customer: google Various Google teams f: material design flutter/packages/flutter/material repository. f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels. P3 Issues that are less important to the Flutter project team-desktop Owned by Desktop platforms team triaged-desktop Triaged by Desktop platforms team
Projects
None yet