lulupointu / vrouter

MIT License
202 stars 38 forks source link

Using gesture to get to a previous page leads to exceptions #164

Open sergeylappo opened 3 years ago

sergeylappo commented 3 years ago

Steps to reproduce:

  1. Open an example app provided in the repo
  2. Tap login
  3. Use the left side of the app to drag it "backwards"
  4. Here it is, app is broken and unusable

OSes This issue was reproduced in an example app, using chrome. In my app it also reproduces with at least Mac OS.

Stacktrace looks something like and repeats many many times:

Error: Cannot hit test a render box that has never been laid out.
The hitTest() method was called on this RenderBox: RenderErrorBox#02fd0 NEEDS-LAYOUT NEEDS-PAINT:
  creator: ErrorWidget-[#7516c] ← Navigator-[LabeledGlobalKey<NavigatorState>#f09f1] ← Builder ← VNavigatorObserverBuilder ← RootVRouterData ← NotificationListener<VWidgetGuardMessageRoot> ← Builder ← _RouterScope ← UnmanagedRestorationScope ← Router<Object> ← IconTheme ← IconTheme ← ⋯
  parentData: <none>
  constraints: MISSING
  size: MISSING
Unfortunately, this object's geometry is not known at this time, probably because it has never been laid out. This means it cannot be accurately hit-tested.
If you are trying to perform a hit test during the layout phase itself, make sure you only hit test nodes that have completed layout (e.g. the node's children, after their layout() method has been called).

App behaves correctly if navigation bar is used instead of gestures

lulupointu commented 3 years ago
Test code sample (the one of VRouter pub.dev example) ```dart import 'package:flutter/material.dart'; import 'package:vrouter/vrouter.dart'; void main() { runApp( VRouter( debugShowCheckedModeBanner: false, // VRouter acts as a MaterialApp mode: VRouterMode.history, // Remove the '#' from the url logs: VLogs.info, // Defines which logs to show, info is the default routes: [ VWidget( path: '/login', widget: LoginWidget(), stackedRoutes: [ ConnectedRoutes(), // Custom VRouteElement ], ), // This redirect every unknown routes to /login VRouteRedirector( redirectTo: '/login', path: r'*', ), ], ), ); } // Extend VRouteElementBuilder to create your own VRouteElement class ConnectedRoutes extends VRouteElementBuilder { static final String profile = 'profile'; static void toProfile(BuildContext context, String username) => context.vRouter.to('/$username/$profile'); static final String settings = 'settings'; static void toSettings(BuildContext context, String username) => context.vRouter.to('/$username/$settings'); @override List buildRoutes() { return [ VNester.builder( // .builder constructor gives you easy access to VRouter data path: '/:username', // :username is a path parameter and can be any value widgetBuilder: (_, state, child) => MyScaffold( child, currentIndex: state.names.contains(profile) ? 0 : 1, ), nestedRoutes: [ VWidget( path: profile, name: profile, widget: ProfileWidget(), ), VWidget( path: settings, name: settings, widget: SettingsWidget(), // Custom transition buildTransition: (animation, ___, child) { return ScaleTransition( scale: animation, child: child, ); }, ), ], ), ]; } } class LoginWidget extends StatefulWidget { @override _LoginWidgetState createState() => _LoginWidgetState(); } class _LoginWidgetState extends State { String name = 'bob'; final _formKey = GlobalKey(); @override Widget build(BuildContext context) { return Material( child: Center( child: Column( mainAxisSize: MainAxisSize.min, children: [ Row( mainAxisSize: MainAxisSize.min, crossAxisAlignment: CrossAxisAlignment.center, children: [ Text('Enter your name to connect: '), Container( width: 200, decoration: BoxDecoration( border: Border.all(color: Colors.black), ), child: Form( key: _formKey, child: TextFormField( textAlign: TextAlign.center, onChanged: (value) => name = value, initialValue: 'bob', ), ), ), ], ), SizedBox( height: 20, ), // This FAB is shared and shows hero animations working with no issues FloatingActionButton( heroTag: 'FAB', onPressed: () { setState(() => (_formKey.currentState!.validate()) ? ConnectedRoutes.toProfile(context, name) : null); }, child: Icon(Icons.login), ) ], ), ), ); } } class MyScaffold extends StatelessWidget { final Widget child; final int currentIndex; const MyScaffold(this.child, {required this.currentIndex}); @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar( title: Text('You are connected'), ), bottomNavigationBar: BottomNavigationBar( currentIndex: currentIndex, items: [ BottomNavigationBarItem( icon: Icon(Icons.person_outline), label: 'Profile'), BottomNavigationBarItem( icon: Icon(Icons.info_outline), label: 'Info'), ], onTap: (int index) { // We can access this username via the path parameters final username = VRouter.of(context).pathParameters['username']!; if (index == 0) { ConnectedRoutes.toProfile(context, username); } else { ConnectedRoutes.toSettings(context, username); } }, ), body: child, // This FAB is shared with login and shows hero animations working with no issues floatingActionButton: FloatingActionButton( heroTag: 'FAB', onPressed: () => VRouter.of(context).to('/login'), child: Icon(Icons.logout), ), ); } } class ProfileWidget extends StatefulWidget { @override _ProfileWidgetState createState() => _ProfileWidgetState(); } class _ProfileWidgetState extends State { int count = 0; @override Widget build(BuildContext context) { // VNavigationGuard allows you to react to navigation events locally return VWidgetGuard( // When entering or updating the route, we try to get the count from the local history state // This history state will be NOT null if the user presses the back button for example afterEnter: (context, __, ___) => getCountFromState(context), afterUpdate: (context, __, ___) => getCountFromState(context), child: Padding( padding: const EdgeInsets.symmetric(horizontal: 20.0), child: Center( child: Column( mainAxisSize: MainAxisSize.min, children: [ TextButton( onPressed: () { VRouter.of(context).to( context.vRouter.url, isReplacement: true, historyState: {'count': '${count + 1}'}, ); setState(() => count++); }, child: Container( decoration: BoxDecoration( borderRadius: BorderRadius.circular(50), color: Colors.blueAccent, ), padding: EdgeInsets.symmetric(horizontal: 20.0, vertical: 8.0), child: Text( 'Your pressed this button $count times', style: buttonTextStyle, ), ), ), SizedBox(height: 20), Text( 'This number is saved in the history state so if you are on the web leave this page and hit the back button to see this number restored!', style: textStyle, textAlign: TextAlign.center, ), ], ), ), ), ); } void getCountFromState(BuildContext context) { setState(() { count = (VRouter.of(context).historyState['count'] == null) ? 0 : int.tryParse(VRouter.of(context).historyState['count'] ?? '') ?? 0; }); } } class SettingsWidget extends StatelessWidget { @override Widget build(BuildContext context) { return Padding( padding: const EdgeInsets.symmetric(horizontal: 20.0), child: Center( child: Column( mainAxisSize: MainAxisSize.min, children: [ Text( 'Did you see the custom animation when coming here?', style: textStyle.copyWith(fontSize: textStyle.fontSize! + 2), ), ], ), ), ); } } final textStyle = TextStyle(color: Colors.black, fontSize: 16); final buttonTextStyle = textStyle.copyWith(color: Colors.white); ```

Test setup: vrouter: v1.2.0+14 Chrome - Linux

Important Note: Since I used Linux, I had to change the source code of VRouter to force VCupertinoPage to be used (like it is on Mac). This is the only difference between Mac and Linux inside VRouter so it should do the trick.

Video of the reproduced steps https://user-images.githubusercontent.com/5128921/140401189-0d0d0d65-00f1-4752-8526-74a3f218d4ce.mp4

As you can see, this works for me. Can you tell me if you have the issue when using precisely the same setup as me (apart from using a Mac)? If so, I will get a hold on a Mac and try again using this setup.

sergeylappo commented 3 years ago

Example had: vrouter: ^1.2.0+7 But upgrading the example fixed it.

Unfortunately, my app is already using v1.2.0+14.

Would investigate how to trigger it without my app.

lulupointu commented 3 years ago

I know VRouter had an issue with popping pages at some point. I even remember an issue with VCupertinoPage popping at some point.

For example the CHANGELOG for v1.2.0+13 is Dynamically moving VNester.child around the widget tree could cause popping issues. While it does explicitly mention VCupertinoPage it is possible that this was included.

Nothing 100% sure though, so be sure to let me know if you can reproduce the bug with the latest version !

sergeylappo commented 3 years ago

Haha! Was able to reproduce, cool bug, for sure: 100% reproduction if mouse gets out of the window during the gesture.

And never reproduces in other cases...

It took quite a while to figure out while my reproduction rate dropped from 50% to near never :D

Seems like I'm less nervous in the late evening 🤣

https://user-images.githubusercontent.com/9092362/140412957-e6cb0d4c-11d2-423b-ab82-dfa030285617.mov

lulupointu commented 3 years ago

I can reproduce this issue. I understand why the issue happen. Applying the fix breaks other things so I will need some time to see exactly what's going on.

Thanks for the nicely written issue and that's quite impressive that you found that so specific bug ^^

sergeylappo commented 3 years ago

Thanks for the quick reply and triage.

It’s not an urgent issue at all, but the worst thing - there’s currently no workaround without disabling the gesture. And this leads to an app death, so hopefully it’ll get its fix once.