karvulf / flutter-reorderable-grid-view

BSD 3-Clause "New" or "Revised" License
158 stars 23 forks source link

[bug] Does not work in a TabBarView #78

Closed isenbj closed 2 weeks ago

isenbj commented 1 year ago

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 commented 1 year ago

Hello @isenbj

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

isenbj commented 1 year ago

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

karvulf commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 11 months ago

@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 commented 10 months ago

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

emavgl commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 2 weeks ago

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