Dennis-Krasnov / Flutter-Deep-Link-Navigation

Elegant abstraction for complete deep linking navigation in Flutter
https://pub.dev/packages/deep_link_navigation
MIT License
68 stars 9 forks source link

Navigation animations #3

Open adrianaxente opened 4 years ago

adrianaxente commented 4 years ago

Hello Dennis,

Is there an easy way to work around the temporary limitation of your library regarding route transitions ?

Do you you have any news about the progress of implementation of the route animations ?

Thank you very much

Best regards

Dennis-Krasnov commented 4 years ago

Hi Adrian,

TLDR: I'd have to modify the library a bit.

The reason why I haven't implemented route animations yet is because I haven't needed them for my own app. But it's not that big of a change, and I guess I have a reason to implement it now.

Here's the culprit:

final pageRoute = actionWasPush
        ? MaterialPageRoute<T>(builder: (BuildContext context) => widget) // this would have to be changed
        : NoAnimationPageRoute<T>(builder: (BuildContext context) => widget);

My idea for implementing route transition animations was to make it an optional field in the path and value functions. If not specified, it should default to the material page route (also I think it would be chaotic if sub navigations inherited transitions).

Option 1

Using an enum (maybe integrating https://pub.dev/packages/page_transition) would make it very easy, but won't let you define your own transitions which I think is a limiting.

..path<LibraryDL>((route) => LibraryPage(), transition: PageTransitionType.scale)

Option 2

A much more extensible option could be to always require specifying the page route. This way custom transitions could be easily be integrated, but makes every deep link be more verbose whether it uses a custom transition or not.

// Expects `PageRouteBuilder` instead of generic widget
..path<LibraryDL>((route) => MaterialPageRoute(builder: (_) => LibraryPage()))
..path<LibraryDL>((route) => CustomPageRoute(builder: (_) => LibraryPage(), speed: 2))

// Can maybe make a static extension methods on Widget to make syntax more terse?
..path<LibraryDL>((route) => LibraryPage().materialTransition())
..path<LibraryDL>((route) => LibraryPage().customTransition(speed: 2))

Option 3

Maybe a compromise, maybe unnecessary complexity, but adding special transitionPath and transitionValue functions might help with cutting the verbosity where no special transition is needed.

..path<LibraryDL>((route) => LibraryPage())
// Expects `PageRouteBuilder` instead of generic widget
..pathTransition<LibraryDL>((route) => CustomPageRoute(builder: (_) => LibraryPage(), speed: 2))

I don't want to make too many breaking changes, but I'd love your feedback on the ideas :)

adrianaxente commented 4 years ago

Hi Adrian,

TLDR: I'd have to modify the library a bit.

The reason why I haven't implemented route animations yet is because I haven't needed them for my own app. But it's not that big of a change, and I guess I have a reason to implement it now.

Here's the culprit:

final pageRoute = actionWasPush
        ? MaterialPageRoute<T>(builder: (BuildContext context) => widget) // this would have to be changed
        : NoAnimationPageRoute<T>(builder: (BuildContext context) => widget);

My idea for implementing route transition animations was to make it an optional field in the path and value functions. If not specified, it should default to the material page route (also I think it would be chaotic if sub navigations inherited transitions).

Option 1

Using an enum (maybe integrating https://pub.dev/packages/page_transition) would make it very easy, but won't let you define your own transitions which I think is a limiting.

..path<LibraryDL>((route) => LibraryPage(), transition: PageTransitionType.scale)

Option 2

A much more extensible option could be to always require specifying the page route. This way custom transitions could be easily be integrated, but makes every deep link be more verbose whether it uses a custom transition or not.

// Expects `PageRouteBuilder` instead of generic widget
..path<LibraryDL>((route) => MaterialPageRoute(builder: (_) => LibraryPage()))
..path<LibraryDL>((route) => CustomPageRoute(builder: (_) => LibraryPage(), speed: 2))

// Can maybe make a static extension methods on Widget to make syntax more terse?
..path<LibraryDL>((route) => LibraryPage().materialTransition())
..path<LibraryDL>((route) => LibraryPage().customTransition(speed: 2))

Option 3

Maybe a compromise, maybe unnecessary complexity, but adding special transitionPath and transitionValue functions might help with cutting the verbosity where no special transition is needed.

..path<LibraryDL>((route) => LibraryPage())
// Expects `PageRouteBuilder` instead of generic widget
..pathTransition<LibraryDL>((route) => CustomPageRoute(builder: (_) => LibraryPage(), speed: 2))

I don't want to make too many breaking changes, but I'd love your feedback on the ideas :)

Tank you you very much Dennis for your answer, For me Option 2 with an extension method looks elegant enough.

Keep up your good work.

Best regards

adrianaxente commented 4 years ago

Hello Dennis,

I am reopening this item because I fully implemented the page transition in my forked repository. I will ask you to review my pull request and after everything is ok maybe integrate it to your main repo.

Best Regards, Adrian