Refactor/ navigation remake attempt #118
Conversation
simon-the-shark
force-pushed
the
refactor/navigation-remake
branch
from
eece632 to
cfe9917
Compare
|
NOTE: #130 reverses detail views transition builder to default one |
There was a problem hiding this comment.
Excellent work here. I'm not a navigation Sensei here but even for me (beginner in complex Flutter's concepts, eg. navigation 🙃) everything looks nice and cool. Personally I didn't notice sth strange.
I checked the auto_route lib, excellent in docs and easy-to-use. Wejust need to add annotation and configure route in controller... Good choice I guess. WDYT about updating readme? Even with some refrence to docs, like with internalization module.
In my opinion the nav stack in typical scenario won't be extremely large. So this should not be the problem. Even Android Studio emulator did well.
We could always think of adding feature for tracking app performance and watch the behaviour on prod. Once I read sth about it here. Wonder what u think ;)
lib/config/transitions.dart
Outdated
| abstract class TransitionsConfig { | ||
| static const durationInMiliseconds = 200; | ||
| static const detailDurationInMiliseconds = 200; | ||
| static const slideLeftBuilder = TransitionsBuilders.slideLeftWithFade; | ||
| static const slideRightBuilder = TransitionsBuilders.slideRightWithFade; | ||
| static const fallbackBuilder = TransitionsBuilders.fadeIn; | ||
| static const detailViewBuilder = TransitionsBuilders.slideBottom; | ||
| } |
There was a problem hiding this comment.
Actually very good that they're parametrized in one section. Imo UX's nice.
| @@ -73,7 +70,7 @@ class _DataListBuildingsTiles extends ConsumerWidget { | |||
| buildingName: mapItem.name, | |||
| imageUrl: mapItem.cover?.filename_disk?.directusUrl, | |||
| onTap: () async { | |||
| await BuildingsSection.goToMapTab(ref); | |||
| unawaited(ref.navigateBuildings()); | |||
There was a problem hiding this comment.
unawaited here's very very nice since we don't use the Future
There was a problem hiding this comment.
Linter screams if you don't do it :) now you have to either await the future or unwait
| @@ -0,0 +1,47 @@ | |||
| import "package:flutter_riverpod/flutter_riverpod.dart"; | |||
There was a problem hiding this comment.
Initially I was wondering "from where the ref.navigateHome() come from" extremely useful extension here.
There was a problem hiding this comment.
Yeah, I've been playing around with different approaches and this in one place was usefull when making any changes
|
Thanks for checking out the changes. Readme link is a good idea too. And about the navigation stack, let's hope for now that flutter handle it well on its own. And yeah, performance and crashlytics are probably a must have before production |
Preview
WhatsApp.Video.2024-08-01.at.22.39.40.mp4
PR Desc
Small remake of the navigation (yet again:))
auto_routeand other small refactoring.tell me what you think