Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug] Does not work in a TabBarView #78

Closed
isenbj opened this issue Jun 21, 2023 · 12 comments · Fixed by #147
Closed

[bug] Does not work in a TabBarView #78

isenbj opened this issue Jun 21, 2023 · 12 comments · Fixed by #147
Labels
bug Something isn't working

Comments

@isenbj
Copy link

isenbj commented Jun 21, 2023

I modified the example project code as shown below, to just wrap it into a TabBarView since my app has tabs, each showing a grid of images. The behavior goes all crazy on me when I do this though. The grid reloads each time an entity is reordered, and the scrolling is not registered, so the reordering no longer works when scrolling down. See the video shared below.

Edit: I think this may have something to do with the GlobalHey. The TabBarView uses an animation so it is creating the new grid before the new one is disposed of. This may be whats causing the issue but am unsure.

@override
  Widget build(BuildContext context) {
    return DefaultTabController(
        animationDuration: Duration(milliseconds: 200),
        length: 3,
        child: Builder(builder: (context) {
          return Scaffold(
            appBar: AppBar(
              flexibleSpace: SafeArea(
                child: TabBar(
                  enableFeedback: true,
                  tabs: [
                    Tab(text: 'tab 1', key: UniqueKey(),),
                    Tab(text: 'tab 2', key: UniqueKey(),),
                    Tab(text: 'tab 3', key: UniqueKey(),),
                  ],
                ),
              ),
            ),
            backgroundColor: Colors.white70,
            body: SafeArea(
              child: Padding(
                padding: const EdgeInsets.fromLTRB(20, 20, 20, 0),
                child: Column(
                  children: [
                    ChangeChildrenBar(
                      onTapAddChild: () {
                        setState(() {
                          // children = children..add(keyCounter++);
                          children.insert(0, keyCounter++);
                        });
                      },
                      onTapRemoveChild: () {
                        if (children.isNotEmpty) {
                          setState(() {
                            // children = children..removeLast();
                            children.removeAt(0);
                          });
                        }
                      },
                      onTapClear: () {
                        if (children.isNotEmpty) {
                          setState(() {
                            children = <int>[];
                          });
                        }
                      },
                      onTapUpdateChild: () {
                        if (children.isNotEmpty) {
                          children[0] = 999;
                          setState(() {
                            children = children;
                          });
                        }
                      },
                      onTapSwap: () {
                        _handleReorder([
                          const OrderUpdateEntity(oldIndex: 0, newIndex: 1),
                        ]);
                      },
                    ),
                    DropdownButton<ReorderableType>(
                      value: reorderableType,
                      icon: const Icon(Icons.arrow_downward),
                      iconSize: 24,
                      elevation: 16,
                      itemHeight: 60,
                      underline: Container(
                        height: 2,
                        color: Colors.white,
                      ),
                      onChanged: (ReorderableType? reorderableType) {
                        setState(() {
                          _scrollController = ScrollController();
                          _gridViewKey = GlobalKey();
                          this.reorderableType = reorderableType!;
                        });
                      },
                      items: ReorderableType.values.map((e) {
                        return DropdownMenuItem<ReorderableType>(
                          value: e,
                          child: Text(e.toString()),
                        );
                      }).toList(),
                    ),
                    const SizedBox(height: 20),
                    Expanded(
                      child: TabBarView(
                        key: UniqueKey(),
                        physics: const NeverScrollableScrollPhysics(),
                        children: <Widget>[
                          // show all images
                          _getReorderableWidget(),
                          // show building images
                          Container(),
                          // show personal property images
                          Container(),
                        ],
                      ),
                    ),
                    // Expanded(child: _getReorderableWidget()),
                  ],
                ),
              ),
            ),
          );
        }));
  }

https://github.com/karvulf/flutter-reorderable-grid-view/assets/49786150/b3718a1d-bb36-4080-bd3e-5fa963c0a5aa

@karvulf
Copy link
Owner

karvulf commented Jun 22, 2023

Hello @isenbj

I can check that. Which version of this package do you use?

@karvulf karvulf added bug Something isn't working question Further information is requested Waiting for response Waiting for an answer of the person who opened the issue labels Jun 22, 2023
@isenbj
Copy link
Author

isenbj commented Jun 22, 2023

This happens with the latest, or with the branch for 5.0.0

@karvulf karvulf removed question Further information is requested Waiting for response Waiting for an answer of the person who opened the issue labels Jun 22, 2023
@karvulf
Copy link
Owner

karvulf commented Jun 22, 2023

So I inspected your problem and I found two things:
The first one, you shouldn't set the key for your TabBarView directly in your code because after a render, a new key will be created which will recreate all the children like ´ReorderableBuilder. So if you need this key, you should declare it at the top of your widget. The second one seems more problematic. Because when I added the TabBar, ReorderableBuilder` is reordering the children wrongly when scrolling down. I will investigate that issue a bit more and try to release a fix as soon as possible. @isenbj

@isenbj
Copy link
Author

isenbj commented Jun 22, 2023

Thanks for looking into it!

Also note that for the children of the TabBarView I just used empty containers for the other tabs:

  children: <Widget>[
      // tab 0
     _getReorderableWidget(),
      // tab 1
      Container(),
       // tab 2
       Container(),
    ],
),

If I try and create another reorderable grid in the other tabs, I run into issues with a duplicate GlobalKey.

I can create a custom stateful widget, and create different global keys at the widget level, but this runs into issues with the TabBarView Creating the new widget before disposing the old one. So the keys are still there. If I remove the animation between tabs then it works fine because it is disposed first.

Alternatively I can create different GlobalKeys and use different ones per grid, but of course this creates issues due to the nature of GlobalKeys.

Is the GlobalKey the only way to make scrolling work?

I have another use case where I would like two different grids on the same screen (the screen is a tablet, so two fit well) but since the GlobalKey is needed, I cannot do this.

@karvulf
Copy link
Owner

karvulf commented Jun 23, 2023

Currently you need a unique key for the GridView. I would say it should work as long as the keys are different. What kind of other issues do you have if each GridView gets another key? @isenbj

@isenbj
Copy link
Author

isenbj commented Jun 23, 2023

I've went a different route in my project so I do not need the two grids anymore, and in my TabView I just use the same widget and update the grid elements vs a new grid which seems to solve my problems. I can't use TabBarView still due to the weird behavior between tabs with scrolling, but I just made my own custom view which is working.

I would like to go back to the TabBarView eventually though, but my current solution is working.

@kathayatnk
Copy link

@karvulf any updates on the fixed. Used the latest version of this package and adding the ReorderableBuilder withing TabBarView the ordering doesn't work anymore.

@karvulf
Copy link
Owner

karvulf commented Jan 9, 2024

Hello @kathayatnk, unfortunately I didn't make any further progress on that

@emavgl
Copy link

emavgl commented Nov 8, 2024

Hi @karvulf - still not in the plan? I also have the same problem. I put the grid in a tabview and it is pretty unreliable, expecially when dragging and scrolling. It works when scrolling on the visible screen, but when trying to dragging, the Gridview is rebuilt (I believe).

@karvulf
Copy link
Owner

karvulf commented Nov 11, 2024

Hello @emavgl

It's a bit long ago since I checked that issue, I will try to find some time this week and see what I can do, hopefully with a fix :) @emavgl

@karvulf
Copy link
Owner

karvulf commented Nov 11, 2024

Hello @emavgl @kathayatnk and @isenbj
I checked it and it should work if the required parameters are set correctly. That means that each GridView has his own GlobalKey and ScrollController so that the ReorderableBuilder can calculate the right things to the matching GridView.
To make it more clear, here is simplified example, I added. I let this issue open as long as you have issues:

import 'package:flutter/material.dart';
import 'package:flutter_reorderable_grid_view/widgets/widgets.dart';

void main() {
  runApp(const MaterialApp(home: MyApp()));
}

class MyApp extends StatefulWidget {
  const MyApp({super.key});

  @override
  State<MyApp> createState() => _MyAppState();
}

class _MyAppState extends State<MyApp> {
  static const _myTabs = <Tab>[
    Tab(text: 'LEFT'),
    Tab(text: 'RIGHT'),
  ];
  final _gridViewLists = <List<String>>[
    List.generate(100, (index) => 'LEFT_$index'),
    List.generate(100, (index) => 'RIGHT_$index'),
  ];
  final _gridViewKeys = <GlobalKey>[
    GlobalKey(),
    GlobalKey(),
  ];
  final _scrollControllers = [
    ScrollController(),
    ScrollController(),
  ];

  @override
  Widget build(BuildContext context) {
    return DefaultTabController(
      length: _myTabs.length,
      child: Scaffold(
        appBar: AppBar(
          bottom: const TabBar(
            tabs: _myTabs,
          ),
        ),
        body: TabBarView(
          children: _myTabs.map((Tab tab) {
            final index = _myTabs.indexOf(tab);
            final list = _gridViewLists[index];

            final generatedChildren = List.generate(
              list.length,
              (index) => Container(
                key: Key(list.elementAt(index)),
                color: Colors.lightBlue,
                child: Text(
                  list.elementAt(index),
                ),
              ),
            );

            return ReorderableBuilder(
              scrollController: _scrollControllers[index],
              onReorder: (ReorderedListFunction reorderedListFunction) {
                final updatedList = reorderedListFunction(list) as List<String>;
                setState(() {
                  _gridViewLists[index] = updatedList;
                });
              },
              children: generatedChildren,
              builder: (children) {
                return GridView(
                  key: _gridViewKeys[index],
                  controller: _scrollControllers[index],
                  gridDelegate: const SliverGridDelegateWithFixedCrossAxisCount(
                    crossAxisCount: 4,
                    mainAxisSpacing: 4,
                    crossAxisSpacing: 8,
                  ),
                  children: children,
                );
              },
            );
          }).toList(),
        ),
      ),
    );
  }
}

@karvulf
Copy link
Owner

karvulf commented Nov 16, 2024

I close this issue and if it still happens, just reopen this issue. I also merge the new example to the master @emavgl

@karvulf karvulf linked a pull request Nov 16, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants