mt-akar / bottom_nav_layout

A quick and powerful Flutter layout with a bottom navbar.
https://pub.dev/packages/bottom_nav_layout
MIT License
34 stars 11 forks source link

Scroll to top doesn't work #5

Open mark8044 opened 3 years ago

mark8044 commented 3 years ago

Have you noticed that if you have scrollviews or list views in the tabs that tapping on the very top bar on iOS no longer does a scroll to top?

Any solution?

I tried creating unique keys but that didn't seem to do anything

   return BottomNavLayout(
      // The app's destinations
      pages: [
            (navKey) => Navigator(
                          key: UniqueKey(),
                          initialRoute: "/",
                          onGenerateRoute: (routeSettings) => MaterialPageRoute(
                            builder: (context) {
                         return NewsScreen();
                            },
                          ),
                         ),
            (navKey) => Navigator(
                          key: UniqueKey(),
                          initialRoute: "/2",
                          onGenerateRoute: (routeSettings) => MaterialPageRoute(
                            builder: (context) {
                              return SecondScreen();
                            },
                          ),
                        ),
            (navKey) => Navigator(
                          key: UniqueKey(),
                          initialRoute: "/3",
                          onGenerateRoute: (routeSettings) => MaterialPageRoute(
                            builder: (context) {
                              return ThirdScreen();
                            },
                          ),
                        ),

      ],
        savePageState:true,
        lazyLoadPages:true,
      extendBody:false,
      pageStack: ReorderToFrontPageStack(initialPage: 0),
      bottomNavigationBar: (...)
mark8044 commented 3 years ago

From my basic understanding of the issue after some research, its because the top level Scaffold might be getting those clicks and from the graphic you show, this project creates a top level Scaffold and all the pages are underneath that.

Not sure if that's correct, but incase it is, I tried setting PrimaryScrollController.of(context) as my scrollcontroller for the listviews but that still does not correct the issue.

mark8044 commented 3 years ago

Here is a stripped down app to show what I mean

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

  @override
  Widget build(BuildContext context) {

    return  MaterialApp(
        home: LayoutBotNav()
    );

  }
}

class ListViewPage extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        actions: [],
      ),
      body: ListView.builder(
        itemCount: 100,
        itemBuilder: (context, i) => Container(
          child: Text(i.toString()),
        ),
        primary: true,
      ),
    );
  }
}

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

  @override
  Widget build(BuildContext context) {

    return BottomNavLayout(
      // The app's destinations
      pages: [

            (navKey) => Navigator(
                                key: navKey,
                                initialRoute: "/",
                                onGenerateRoute: (routeSettings) => MaterialPageRoute(
                                  builder: (context) {
                                    return ListViewPage();
                                  },
                                ),
                              ),
            (navKey) => Navigator(
                                key: navKey,
                                initialRoute: "/",
                                onGenerateRoute: (routeSettings) => MaterialPageRoute(
                                  builder: (context) {
                                    return Container();
                                  },
                                ),
                              ),

      ],
        savePageState:true,
        lazyLoadPages:true,
      extendBody:false,
      bottomNavigationBar:
          (currentIndex, onTap) => Container(
        BottomNavigationBar(
          backgroundColor: Colors.white,
          currentIndex: currentIndex,
          onTap: (index) => onTap(index),
          items: [
            BottomNavigationBarItem(
                icon: Icon(Icons.list),
                label: 'Tab 1',
            ),
            BottomNavigationBarItem(icon: Icon(Icons.check_box_outline_blank), label: 'Tab 2'),

          ],
          showUnselectedLabels: true,
          type: BottomNavigationBarType.fixed,
        ),
      ),
    );

  }
}
mt-akar commented 3 years ago

I only have done some testing but haven't done extensive testing on iOS mainly due to the fact that I own neither a Mac nor an iPhone. Honestly, I learned about the double tap status bar to scroll up feature from you.

I have a question. Which widget in the Flutter framework provides the behavior you have talked about? This might help me identify the problem.

If it is the CupertinoPageScaffold or CupertinoApp, then I know the problem. This package uses the material Scaffold and MaterialApp widgets. This might be changed and made dynamic upon request but I need to be sure.

mark8044 commented 3 years ago

I only have done some testing but haven't done extensive testing on iOS mainly due to the fact that I own neither a Mac nor an iPhone. Honestly, I learned about the double tap status bar to scroll up feature from you.

I have a question. Which widget in the Flutter framework provides the behavior you have talked about? This might help me identify the problem.

If it is the CupertinoPageScaffold or CupertinoApp, then I know the problem. This package uses the material Scaffold and MaterialApp widgets. This might be changed and made dynamic upon request but I need to be sure.

Im not really too sure, from what I can tell it should still work with a Scaffold. In my example above, I'm using a regular Scaffold and scroll to top works normally when I load those pages without using bottom nav layout.

think Piinks from the flutter team is involved in this functionality, and here are some interesting posts on the topic:

https://github.com/flutter/flutter/issues/85603#issuecomment-876798161 https://github.com/flutter/flutter/issues/74727#issuecomment-813504194

But there are a few other threads on the topic, but I'm unclear on what the issue is

mark8044 commented 3 years ago

Hi, so I'm not sure what the way to fix this problem, but I made my own Cupertino based navigation bar and the scroll to top works perfectly across multiple tabs

Here is the basic navbar I made that works. Of course its missing a lot of the elegant features that come with your package. I'm not exactly sure how to transpose this to your code but take a look, you are smarter then me:

    return WillPopScope(
        onWillPop: () async {
          return !await listOfKeys[tabController.index].currentState!.maybePop();
        },
        child: CupertinoTabScaffold(
          controller: tabController, //set tabController here

          tabBar: CupertinoTabBar(
            items: [
              ///this is where we are setting our bottom ICONS
              BottomNavigationBarItem(
                icon: Icon(FontAwesome5.newspaper),
                label: 'Screen1',
              ),
              BottomNavigationBarItem(icon: Icon(Icons.format_list_bulleted), label: 'Screen2'),
              BottomNavigationBarItem(icon: Icon(Icons.mail_outline_outlined), label: 'Screen3'),
              BottomNavigationBarItem(icon: Icon(Icons.alternate_email), label: 'Screen4'),
              BottomNavigationBarItem(icon: Icon(Icons.more_horiz), label: 'Screen5'),
            ],
            iconSize:22,
            // currentIndex: pageIndex,
          ),
          tabBuilder: (
              context,
              index,
              ) {
            return CupertinoTabView(
              navigatorKey: listOfKeys[
              index], //set navigatorKey here which was initialized before
              builder: (context) {
                return homeScreenList[index];
              },
            );
          },
        ),
      );

I can post the full code if needed

It also seems like Scaffold retains the scroll to function of iOS https://github.com/flutter/flutter/issues/24777 so its not clear to me that it needs to be converted to a Cupertino Scaffold. I have a feeling there is something else happening?

mark8044 commented 2 years ago

An update

I was using bottom nav layout like this:

    return BottomNavLayout(
      // The app's destinations
      pages: [
            (navKey) => Navigator(
                          key: navKey,
                          initialRoute: "/",
                          onGenerateRoute: (routeSettings) => MaterialPageRoute(
                            builder: (context) {
                         return NewsScreen();
                            },
                          ),
                         ),
            (navKey) => Navigator(
                          key: navKey,
                          initialRoute: "/",
                          onGenerateRoute: (routeSettings) => MaterialPageRoute(
                            builder: (context) {
                              return TextbookScreen();
                            },
                          ),
                        ),
            (navKey) => Navigator(
                          key: navKey,
                          initialRoute: "/",
                          onGenerateRoute: (routeSettings) => MaterialPageRoute(
                            builder: (context) {
                              return ArticleScreen();
                            },
                          ),
                        ),
      ],

BUT if I convert to a more basic version

       pages: [
            (_) => NewsScreen(),
            (_) => TextbookScreen(),
            (_) => ArticleScreen(),
      ],

Then scroll to top function works again. But I am getting a million errors in the app related to the scrollcontrollers being shared between the different tabs.

BUT it does work...

Does this give any clues to what the issue is? (changing MaterialPageRoute to CupertinoPageRoute has no effect). From what I can understand is that each route has its own PrimaryScrollController. So why does having all those separate navigators not allow it to work anymore?

mt-akar commented 2 years ago

Interesting find. I am going to have to look more deeper into how scroll controllers work. May I ask, are you using scroll controllers for another purpose than simply scrolling the screen?

I don't want to throw assumptions and hypothesis around without properly looking into this.

mark8044 commented 2 years ago

Interesting find. I am going to have to look more deeper into how scroll controllers work. May I ask, are you using scroll controllers for another purpose than simply scrolling the screen?

I don't want to throw assumptions and hypothesis around without properly looking into this.

I am using the scrollcontrollers mainly for infinite scroll purposes, when a listview is close to the bottom of the screen the next page of data gets loaded.

That all works just fine, no problems at all

    var _scrollController2 = PrimaryScrollController.of(context);
        _scrollController2?.addListener(() async {

          if ((_scrollController2.position.pixels > (_scrollController2.position.maxScrollExtent - 650.0)))
                    {
                          (.... load more data in listview ...)
                  }
        });

This all works great, no problems.

I feel the solution might be something straightforward, Ive been playing with the Navigator part of that code all night but no luck yet

mark8044 commented 2 years ago

@m-azyoksul

Do you think this is the solution?

https://medium.com/@suhw4n/flutter-tab-to-scroll-top-on-nested-navigation-b176cc759921

mt-akar commented 2 years ago

Assuming it is the cause of the problem, the problem makes more sense. It seem a bit to complicated. I can see how you came across this problem and none of us could solve. I think the real solution shoul be on the framework level.

Could you confirm if it solves your problem @mark8044 ? If it does, it would be safe to say that it is the cause. Then I will be relieved that this isn't about this package.

mark8044 commented 2 years ago

Assuming it is the cause of the problem, the problem makes more sense. It seem a bit to complicated. I can see how you came across this problem and none of us could solve. I think the real solution shoul be on the framework level.

Could you confirm if it solves your problem @mark8044 ? If it does, it would be safe to say that it is the cause. Then I will be relieved that this isn't about this package.

Do you have any idea how to implement this with your package? I'm not sure how to interact between this and bottom layout nav

mark8044 commented 2 years ago

I CAN CONFIRM the fix works đź‘Ť
I will leave this here for anyone else who has such a problem.

What I did was place your framework into a stateful widget and assigned global keys for each tab, and used those for each tab navigator

            (navKey) => Navigator(
          key: widget.firstTabNavKey,
          initialRoute: "/",
          onGenerateRoute: (routeSettings) => MaterialPageRoute(
            builder: (context) {
              return NewsScreen();
            },
          ),
        ),

and then inside each screen, I setup my scroll controller as the first step

mark8044 commented 2 years ago

@m-azyoksul Ok just an update, the issue has to do at its core with an outer Scaffold accepting all the clicks on the top bar, I converted the Scaffold to a Stack and everything seems to work really well

layout.dart

    return WillPopScope(
      onWillPop: onWillPop,
      child: Stack(
        //extendBody: widget.extendBody,
        //resizeToAvoidBottomInset: widget.resizeToAvoidBottomInset,
        //body: widget.pageTransitionData == null
        children:[
          widget.pageTransitionData == null
            ? IndexedStack(
                key: ValueKey<int>(widget.savePageState ? 0 : currentIndex),
                index: currentIndex,
                // If the page is not initialized, "not show" an invisible widget instead.
                children:
                    pages.map((page) => page ?? SizedBox.shrink()).toList(),
              )
            : TwoWayAnimatedIndexedStack(
                key: ValueKey<int>(widget.savePageState ? 0 : currentIndex),
                animData: widget.pageTransitionData!,
                index: currentIndex,
                // If the page is not initialized, "not show" an invisible widget instead.
                children:
                    pages.map((page) => page ?? SizedBox.shrink()).toList(),
              ),
            Align(alignment:Alignment.bottomCenter,child:widget.bottomNavigationBar(currentIndex, onPageSelected)),
    ]
      ),
    );

The issues are that the bottom navigation bar overlays some of the content in the main content window and some of the scaffold related properties are now gone like the system ui color for the bottom bar on android, but I think they can be easily overcome. Maybe a Column is better then stack to deal with the overlap

mt-akar commented 2 years ago

@mark8044 I see what you are saying. The reason I choose scaffold as the container is because the framework itself uses a scaffold for this purpose and it has additional features that I use. I can totally refactor the code to use a column or a stack instead but as you said, some additional work would be required.

I marked this as enhancement. I don't have time for this for now but I will work on refactoring this part when I get the chance to. Feel free to submit a PR, that would be great.

Ax0elz commented 1 year ago

Is there any update on this? My scroll to top has not been working for a few months now, including the latest 3.7.3

mt-akar commented 1 year ago

The cause of the issue is identified. It is because the layout uses a scaffold as part of its widget tree. I need to replace the scaffold with a Row and implement the behavior for 2 parameters that were previously handled by the scaffold. I just didn't get a chance to do that. I am committed to fixing this but cannot promise on a timeline.