ueman / feedback

A simple widget for getting better feedback.
https://pub.dev/packages/feedback
392 stars 94 forks source link

Interference with navigation #167

Open ashemetov opened 2 years ago

ashemetov commented 2 years ago

Version

2.3.0

Library

feedback

Flutter channel

stable

Flutter version

2.8.1

Platform

Android, iOS

Details

Thank you for the excellent package! Starting from version 2.3.0 Feedback interferes with navigation - I can not go back to previous screen. Version 2.2.1 is working perfectly. Note that interference maybe with GetX that I am using, but again, previous version had no issues.

Steps to reproduce

Output of flutter doctor -v

No response

ueman commented 2 years ago

Dou you mind linking the package? I alway get confused because there are a couple quite similar named packages. Which version of GetX are you using?

ashemetov commented 2 years ago

I am using get: 4.6.1. I'll try to build a demo for the issue.

ashemetov commented 2 years ago

Steps for repro: Create new flutter project, replace 'lib\main.dart' with following:

import 'package:flutter/material.dart'; import 'package:feedback/feedback.dart'; import 'package:get/get.dart';

void main() { runApp(const BetterFeedback(child: MyApp())); }

class MyApp extends StatelessWidget { const MyApp({Key? key}) : super(key: key);

@override Widget build(BuildContext context) { return GetMaterialApp( title: 'Flutter Demo', theme: ThemeData( primarySwatch: Colors.blue, ), home: const MyHomePage(title: 'Flutter Demo Home Page'), ); } }

class MyHomePage extends StatefulWidget { const MyHomePage({Key? key, required this.title}) : super(key: key);

final String title;

@override State createState() => _MyHomePageState(); }

class _MyHomePageState extends State { void _showDialog() { Get.defaultDialog(title: 'Test', middleText: '', actions: [ ElevatedButton(child: const Text('Yes'), onPressed: () => Get.back()), ElevatedButton(child: const Text('Cancel'), onPressed: () => Get.back()) ]); }

@override Widget build(BuildContext context) { return Scaffold( appBar: AppBar( title: Text(widget.title), ), floatingActionButton: FloatingActionButton( onPressed: _showDialog, child: const Icon(Icons.error), ), ); } }

add following to 'pubspec.yaml':

dependencies: flutter: sdk: flutter feedback: 2.2.1 # works '# feedback: 2.3.0 # doesn't work get: 4.6.1

Run app, click on icon, dialog will be shown, click on any button, dialog will be closed. Change version to 2.3.0, repeat steps - dialog will be not closed.

I am not sure if it interferes with navigation in general or with GetX navigation, however demo shows that issue appeared with new version of Feedback.

phillausofia commented 2 years ago

I get an error

Duplicate GlobalKey detected in widget tree.

When trying to use Navigator.of(context).pushReplacement with version higher than 2.2.1.

ueman commented 2 years ago

I get an error

Duplicate GlobalKey detected in widget tree.

When trying to use Navigator.of(context).pushReplacement with version higher than 2.2.1.

How do you use the BetterFeedback widget? Do you wrap it around the MaterialApp or around a Scaffold?

caleb654 commented 2 years ago

I'm also getting this Duplicate Global key issue. I have tried putting the Better Feedback widget in multiple different places

vinceramcesoliveros commented 2 years ago

I tried below the Material App and above. the errors occured as follows

Above MaterialApp - Dialogs are not working properly(using showAlertDialog) Below MaterialApp - Duplicate GlobalKey

Above Material App also disrupts the bloc or provider's instance.

ilhanaydinli commented 2 years ago

Hello everyone, first of all thanks for your package. We ran into the same problem, and I came up with a few solutions. In order to find the problem more easily, I will share these solutions with you below. We are using the get package in our project. You need to wrap the BetterFeedback on top of GetMaterialApp. For example, if you wrap BetterFeedback under GetMaterialApp, Get.bottomSheet will open above BetterFeedback.

Solution 1: When we wrap GetMaterialApp with BetterFeedback, Get.back() does not work in dialog boxes. But when I tried something like this, I saw that it was working. Navigator.of(Get.context!, rootNavigator: true).pop('dialog');

Solution 2: When we removed the Navigator and MaterialPageRoute at the 142 line in the feedback_widget.dartfile, I saw that both the dialog boxes and BetterFeedback worked fine.

-    return Navigator(
-      onGenerateRoute: (_) {
-        return MaterialPageRoute<void>(
-          builder: (context) {
+           return Material(
RootSoft commented 2 years ago

@ueman thanks for this wonderful package. I seem to be having some navigational issues as well. I'm using a combination of reactive_forms, flow_builder and fluro in my app which uses a lot of underlying navigational intents.

@ilhanaydinli's approach of removing the Navigator and MaterialPageRoute seems to help and fixes some of these navigation issues.

I'll try to create a reproducable example in the following days.

sleewok commented 2 years ago

I am having other

Get.isDialogOpen does not work either. I was wondering why none of my loaders would hide. I check for that before hiding a loader because dismissing it calls Get.back()

sleewok commented 2 years ago

Solution 2 fixed my issue with using Get.isDialogOpen(). Have you noticed any side effects from these changes?

I'm using the custom_feedback.dart example code and get the following error when trying to set the dropdownbutton for feedback_type within the bottomSheet:

Navigator operation requested with a context that does not include a Navigator.

mrgzi commented 2 years ago

I am having the same issue and I dont use GetX package. Exception has occurred. FlutterError (Looking up a deactivated widget's ancestor is unsafe. At this point the state of the widget's element tree is no longer stable. To safely refer to a widget's ancestor in its dispose() method, save a reference to the ancestor by calling dependOnInheritedWidgetOfExactType() in the widget's didChangeDependencies() method.)

mrgzi commented 2 years ago

I'm also getting this Duplicate Global key issue. I have tried putting the Better Feedback widget in multiple different places

Yes because of this it's very struggling to push the same route.

xmine64 commented 2 years ago

it also breaks BuildContext of showDialog(...), LocalizationDelegates and other stuff from my MaterialApp is not accessible when I wrap it with BetterFeedback.

aliyazdi75 commented 2 years ago

@ueman Is this going to be fixed soon? I think this is so critical and the app doesn't work with this problem.

ueman commented 2 years ago

I'm not actively working on that problem, but I would gladly guide or accept a PR which fixes this issue.

aliyazdi75 commented 2 years ago

@ueman The easy solution that inserting for example TestLocalizations.delegate to the BetterFeedback.localizationsDelegates:

      localizationsDelegates: [
        GlobalFeedbackLocalizationsDelegate(),
        TestLocalizations.delegate,
        GlobalMaterialLocalizations.delegate,
        GlobalCupertinoLocalizations.delegate,
        GlobalWidgetsLocalizations.delegate,
      ],

If this solution is ok, I think it's better to mention it in readme or the example project.

mubiru-simeon commented 2 years ago

@ueman Is this going to be fixed soon? I think this is so critical and the app doesn't work with this problem.

Did you find a solution?.. Or a work around for the navigation context issues and the dialog box issues?

aliyazdi75 commented 2 years ago

@ueman The easy solution that inserting for example TestLocalizations.delegate to the BetterFeedback.localizationsDelegates:

      localizationsDelegates: [
        GlobalFeedbackLocalizationsDelegate(),
        TestLocalizations.delegate,
        GlobalMaterialLocalizations.delegate,
        GlobalCupertinoLocalizations.delegate,
        GlobalWidgetsLocalizations.delegate,
      ],

If this solution is ok, I think it's better to mention it in readme or the example project.

@nirancodelab Can't you see this comment?

abhishek199-dhn commented 2 years ago

@ueman The easy solution that inserting for example TestLocalizations.delegate to the BetterFeedback.localizationsDelegates:

      localizationsDelegates: [
        GlobalFeedbackLocalizationsDelegate(),
        TestLocalizations.delegate,
        GlobalMaterialLocalizations.delegate,
        GlobalCupertinoLocalizations.delegate,
        GlobalWidgetsLocalizations.delegate,
      ],

If this solution is ok, I think it's better to mention it in readme or the example project.

@nirancodelab Can't you see this comment?

I'm having the same issue but this solution didn’t work for me.

BetterFeedback(
      ...
      localizationsDelegates: [
          GlobalFeedbackLocalizationsDelegate(),
          CountryLocalizations.delegate,
      ],
      child: MaterialApp(
              navigatorKey: navigatorKey,
              navigatorObservers: <NavigatorObserver>[observer],
              routes: routes,
              ...
              supportedLocales: [
                Locale("en"),
                Locale("es"),
                Locale("id"),
                Locale("sr"),
                Locale("sg")
              ],
              localizationsDelegates: [
                 CountryLocalizations.delegate,
              ],
        ),
);

@aliyazdi75 is there anything else that needs to be done?

aliyazdi75 commented 2 years ago

@abhishek199-dhn Change it like this.

 MaterialApp(
              navigatorKey: navigatorKey,
              navigatorObservers: <NavigatorObserver>[observer],
              routes: routes,
              ...
              supportedLocales: [
                Locale("en"),
                Locale("es"),
                Locale("id"),
                Locale("sr"),
                Locale("sg")
              ],
              localizationsDelegates: CountryLocalizations.delegates,
              ,
              builder: (context, child) {
                return BetterFeedback(
                    ...
                    localizationsDelegates: [
                        GlobalFeedbackLocalizationsDelegate(),
                        ...CountryLocalizations.delegates,
                    ],
                    child: child,
                );
            },
  ),
);
abhishek199-dhn commented 2 years ago

@aliyazdi75 I tried the above changes but didn't work

MaterialApp(
            navigatorKey: navigatorKey,
            navigatorObservers: <NavigatorObserver>[observer],
            home: SplashScreen(),
            routes: routes,
            builder: (context, child) => BetterFeedback(
              feedbackBuilder: (context, onSubmit, scrollController) =>
                  CustomFeedbackForm(
                    onSubmit: onSubmit,
                    scrollController: scrollController,
                  ),
              child: child,
              localizationsDelegates: [
                GlobalFeedbackLocalizationsDelegate(),
                CountryLocalizations.delegate,
              ],
            ),
            supportedLocales: [
              Locale("en"),
              Locale("es"),
              Locale("id"),
              Locale("sr"),
              Locale("sg")
            ],
            localizationsDelegates: [
              CountryLocalizations.delegate,
            ],
          )

Error log:-

====================================================================================================
flutter: ══╡ EXCEPTION CAUGHT BY WIDGETS LIBRARY ╞═══════════════════════════════════════════════════════════
flutter: The following assertion was thrown while finalizing the widget tree:
flutter: Duplicate GlobalKey detected in widget tree.
flutter: The following GlobalKey was specified multiple times in the widget tree. This will lead to parts of
flutter: the widget tree being truncated unexpectedly, because the second time a key is seen, the previous
flutter: instance is moved to the new location. The key was:
flutter: - [GlobalKey#947ba]
flutter: This was determined by noticing that after the widget with the above global key was moved out of its
flutter: previous parent, that previous parent never updated during this frame, meaning that it either did
flutter: not update at all or updated before the widget was moved, in either case implying that it still
flutter: thinks that it should have a child with that global key.
flutter: The specific parent that did not update after having one or more children forcibly removed due to
flutter: GlobalKey reparenting is:
flutter: - Screenshot
flutter: A GlobalKey can only be specified on one widget at a time in the widget tree.
flutter:
flutter: When the exception was thrown, this was the stack:
flutter: #0      BuildOwner.finalizeTree.<anonymous closure> (package:flutter/src/widgets/framework.dart:2845:15)
flutter: #1      BuildOwner.finalizeTree (package:flutter/src/widgets/framework.dart:2870:8)
flutter: #2      WidgetsBinding.drawFrame (package:flutter/src/widgets/binding.dart:875:19)
flutter: #3      RendererBinding._handlePersistentFrameCallback (package:flutter/src/rendering/binding.dart:319:5)
flutter: #4      SchedulerBinding._invokeFrameCallback (package:flutter/src/scheduler/binding.dart:1144:15)
flutter: #5      SchedulerBinding.handleDrawFrame (package:flutter/src/scheduler/binding.dart:1082:9)
flutter: #6      SchedulerBinding._handleDrawFrame (package:flutter/src/scheduler/binding.dart:998:5)
flutter: #10     _invoke (dart:ui/hooks.dart:161:10)
flutter: #11     PlatformDispatcher._drawFrame (dart:ui/platform_dispatcher.dart:253:5)
flutter: #12     _drawFrame (dart:ui/hooks.dart:120:31)
flutter: (elided 3 frames from dart:async)
flutter: ════════════════════════════════════════════════════════════════════════════════════════════════════
aliyazdi75 commented 2 years ago

@abhishek199-dhn I don't understand why you don't copy paste from my code !!! Use this:

                    localizationsDelegates: [
                        GlobalFeedbackLocalizationsDelegate(),
                        ...CountryLocalizations.delegates,
                    ],

not

              localizationsDelegates: [
                GlobalFeedbackLocalizationsDelegate(),
                CountryLocalizations.delegate,
              ], 
            ...
            localizationsDelegates: [
              CountryLocalizations.delegate,
            ],

If this doesn't work, idk what is your problem.

mubiru-simeon commented 2 years ago

@ueman The easy solution that inserting for example TestLocalizations.delegate to the BetterFeedback.localizationsDelegates:

      localizationsDelegates: [
        GlobalFeedbackLocalizationsDelegate(),
        TestLocalizations.delegate,
        GlobalMaterialLocalizations.delegate,
        GlobalCupertinoLocalizations.delegate,
        GlobalWidgetsLocalizations.delegate,
      ],

If this solution is ok, I think it's better to mention it in readme or the example project.

@nirancodelab Can't you see this comment?

I can. I tried it but it still doesn't work. Dialog boxes can't be popped with back button.

Here's my code

BetterFeedback( localizationsDelegates: [ GlobalFeedbackLocalizationsDelegate(), GlobalMaterialLocalizations.delegate, GlobalCupertinoLocalizations.delegate, GlobalWidgetsLocalizations.delegate, ], child: MaterialApp( navigatorKey: navigatorKey, debugShowCheckedModeBanner: false, title: capitalizedAppName, theme: ThemeData( brightness: brightness, floatingActionButtonTheme: FloatingActionButtonThemeData( foregroundColor: Colors.white, backgroundColor: altColor, ), inputDecorationTheme: InputDecorationTheme( contentPadding: EdgeInsets.only( top: 10, bottom: 10, left: 10, right: 10, ), border: OutlineInputBorder( borderRadius: standardBorderRadius, ), ), primarySwatch: primaryColor, ), home: SplashScreenView(), ), ),

mrverdant13 commented 1 year ago

I am also having navigator-related issues. Looking at the source code, I see a Navigagor is used, but it is not that clear to me why is it required. The troubles I am facing are related to nested navigation and dialogs with un-scoped contexts when used with the root navigator.

I would like to solve this by removing the Navigator, but I'd like to understand the reason for its use. Could you please help me with that @ueman ?

ueman commented 1 year ago

As far as I remember it’s needed for the bottom sheet. But if you can get rid of the navigator, that would be awesome. I‘ve tried a couple of times but had no success so far

mrverdant13 commented 1 year ago

Oh, I noticed that (thanks to an explaining commend there). I guess the approach I might take is to emulate the bottom sheet behaviour, so we will no longer need the Navigator. I will post an update if I come up with a potential solution.

ziadsarour commented 1 year ago

Hope it will help someone : I have fixed my navigation issue with useRootNavigator: false in showGeneralDialog

loph3xertoi commented 11 months ago

Hope it will help someone : I have fixed my navigation issue with useRootNavigator: false in showGeneralDialog

You save me, thank you guy.

starshipcoder commented 11 months ago

Use useRootNavigator: false for a specific showDialog fix the problem

But if I have dialog inside a library, how to do ?

ubergaijin commented 10 months ago

Seems like what TextField only needs is Overlay. I solved all nested navigation conflicts by simply replacing in feedback_widget.dart:

Navigator(
      onGenerateRoute: (_) {
        return MaterialPageRoute<void>(
          builder: (context) ...

with:

Overlay(
      initialEntries: [
        OverlayEntry(builder: (context) ...

Could such fix be implemented? Unless I'm missing another usecase where Navigator is necessary.