jonataslaw / getx

Open screens/snackbars/dialogs/bottomSheets without context, manage states and inject dependencies easily with Get.
MIT License
10.39k stars 1.62k forks source link

Cannot open page again in bottom sheet. #1332

Open Owenli0202 opened 3 years ago

Owenli0202 commented 3 years ago

Describe the bug Cannot open page again in bottom sheet. When I open /Page2 in the bottom sheet, return to bottom sheet and cannot open again / Page2. I found that it is because the didPop in route_observer determines that the route type is not PageRoute, resulting in the currentRoute not being updated.

Reproduction code NOTE: THIS IS MANDATORY, IF YOUR ISSUE DOES NOT CONTAIN IT, IT WILL BE CLOSED PRELIMINARY)

example: void showBottomSheet() { Get.bottomSheet(Container( height: 100, child: InkWell( onTap: () => Get.toNamed('/Page2'), child: Text('open new page'), ), )); }

To Reproduce Steps to reproduce the behavior:

  1. Get.bottomSheet Open the bottom sheet
  2. Click the button in the bottom sheet to call Get.toNamed ('/Page2');
  3. Return to bottom sheet
  4. Click the button in the bottom sheet again to call Get.toNamed ('/Page2');
  5. Cannot open '/ Page2'

Flutter Version: 2.0.5

Getx Version: 3.26.0

Describe on which device you found the bug: Android.

Minimal reproduce code Provide a minimum reproduction code for the problem

XHStudios commented 3 years ago

same question

padgithub commented 3 years ago

same bug

TykanN commented 3 years ago

I reproduced too. It can be solved temporarily by setting preventDuplicates option to false.

But, I think it's not prefect solution. At the bottom sheet, go to new page(/new) and return to bottom sheet through Get.back(); GetX.currentRoute is still /new although i closed page "/new" It should be fixed on route observer.

jja08111 commented 2 years ago

Same issue

jja08111 commented 2 years ago

I created a test code that must be passed but is not.

Code

testWidgets("Get.bottomSheet and Get.back close test", (tester) async {
    await tester.pumpWidget(
      Wrapper(child: Container()),
    );

    Get.bottomSheet(Container(
      child: Wrap(
        children: <Widget>[
          ListTile(
            leading: Icon(Icons.music_note),
            title: Text('Music'),
            onTap: () {},
          ),
        ],
      ),
    ));
    await tester.pumpAndSettle();

    Get.to(First());
    await tester.pumpAndSettle();

    Get.dialog(YourDialogWidget());
    await tester.pumpAndSettle();

    Get.back();
    await tester.pumpAndSettle();

    expect(find.byType(First), findsOneWidget);

    Get.back();
    await tester.pumpAndSettle();

    expect(find.byType(First), findsNothing);
    expect(Get.isBottomSheetOpen, true);

    Get.to(First());
    await tester.pumpAndSettle();

    expect(find.byType(First), findsOneWidget); // <---- Fail
  });

Result

══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure object was thrown running a test:
  Expected: exactly one matching node in the widget tree
  Actual: _WidgetTypeFinder:<zero widgets with type "First" (ignoring offstage widgets)>
   Which: means none were found but one was expected

When the exception was thrown, this was the stack:
#4      main.<anonymous closure> (file:///Users/minseongkim/Documents/GitHub/getx/test/navigation/dialog_test.dart:84:5)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

This was caught by the test expectation on the following line:
  file:///Users/minseongkim/Documents/GitHub/getx/test/navigation/dialog_test.dart line 84
The test description was:
  Get.dialog and Get.back close test
════════════════════════════════════════════════════════════════════════════════════════════════════

I think there is needed to fix the algorithm of the RouteObservere.didPop() function.

jja08111 commented 2 years ago

@jonataslaw Can I ask why the value that is not PageRoute isn't allowed to set the value.current and value.previous?
Becuase I think there are solutions by allowing the value that isn't PageRoute to set the value. Or create a secondaryPreviouse value in the _RouteData to fix this issue.

https://github.com/jonataslaw/getx/blob/b8d2cdbc339569e4bb9e2a79422d5cb4f03f5066/lib/get_navigation/src/routes/observers/route_observer.dart#L61-L68

https://github.com/jonataslaw/getx/blob/b8d2cdbc339569e4bb9e2a79422d5cb4f03f5066/lib/get_navigation/src/routes/observers/route_observer.dart#L102-L105

https://github.com/jonataslaw/getx/blob/b8d2cdbc339569e4bb9e2a79422d5cb4f03f5066/lib/get_navigation/src/routes/observers/route_observer.dart#L165-L168

hashinclude72 commented 1 year ago

any update?

Zaveri21 commented 5 months ago

@Owenli0202 , @hashinclude72 , @jja08111 , @XHStudios

You have to pass params preventDuplicates with value false.

Get.toNamed('/ScreenName', preventDuplicates:false);

try Get.toNamed ('/Page2', preventDuplicates: false,)

Duplicate of #514 & #898