karvulf / flutter-reorderable-grid-view

BSD 3-Clause "New" or "Revised" License
143 stars 20 forks source link

Item rebuild on drag start #85

Open gmarizy opened 1 year ago

gmarizy commented 1 year ago

At drag start, the selected item experience a rebuild. In my case I use this lib to reorder photos, meaning rebuild is a bit expensive and cause a glitch. I tried playing with keys to prevent this glitch to no avail. I think the problem come from flutter-reorderable-grid-view but I am not totally sure about it.

Following is a slightly modified version of the sample of home to illustrate the problem. The print statement line 81 shows that item build method is called on drag start.

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

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

  @override
  _MyAppState createState() => _MyAppState();
}

class _MyAppState extends State<MyApp> {

  final _fruits = <String>["apple", "banana", "strawberry"];

  @override
  Widget build(BuildContext context) {
final fruits = ValueNotifier(["apple", "banana", "strawberry"]);

    return Scaffold(
      body: _Body(fruits: _fruits),
    );
  }
}

class _Body extends StatelessWidget {
  const _Body({
    super.key,
    required List<String> fruits,
  }) : _fruits = fruits;

  final List<String> _fruits;

  @override
  Widget build(BuildContext context) {
    final scrollController = ScrollController();
    final gridViewKey = GlobalKey();

    final generatedChildren = List.generate(
      _fruits.length,
          (index) => NewWidget(fruits: _fruits,   index: index,      key: Key(_fruits.elementAt(index)),),
    );

    return ReorderableBuilder(
      children: generatedChildren,
      scrollController: scrollController,
      onReorder: (List<OrderUpdateEntity> orderUpdateEntities) {
        for (final orderUpdateEntity in orderUpdateEntities) {
          final fruit = _fruits.removeAt(orderUpdateEntity.oldIndex);
          _fruits.insert(orderUpdateEntity.newIndex, fruit);
        }
      },
      builder: (children) {
        return GridView(
          key: gridViewKey,
          controller: scrollController,
          children: children,
          gridDelegate: const SliverGridDelegateWithFixedCrossAxisCount(
            crossAxisCount: 4,
            mainAxisSpacing: 4,
            crossAxisSpacing: 8,
          ),
        );
      },
    );
  }
}

class NewWidget extends StatelessWidget {
  const NewWidget({
    super.key,
    required List<String> fruits,
    required this.index
  }) : _fruits = fruits;

  final int index;
  final List<String> _fruits;

  @override
  Widget build(BuildContext context) {
    print("build index: $index");
    return Container(
        color: Colors.lightBlue,
        child: Text(
    _fruits.elementAt(index),
        ),
      );
  }
}
karvulf commented 1 year ago

Hello @gmarizy You should never put the globalkey and scrollcontroller to the build method Instead put this at the top of your widget Otherwise the ReorderableBuilder will be rebuilt after every render

gmarizy commented 1 year ago

@karvulf Thank you for your fast answer. I removed the globalkey and scrollcontroller from the build method but the issue persist. This a the sample from the readme, with only two modifications; I extracted _Item and added a print statement in it's build method. When I start dragging, there is print meaning _Item is rebuild.

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

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

  @override
  _MyAppState createState() => _MyAppState();
}

class _MyAppState extends State<MyApp> {
  final _scrollController = ScrollController();
  final _gridViewKey = GlobalKey();
  final _fruits = <String>["apple", "banana", "strawberry"];

  @override
  Widget build(BuildContext context) {
    final generatedChildren = List.generate(
      _fruits.length,
          (index) => _Item(
        fruits: _fruits,
        index: index,
        key: Key(_fruits.elementAt(index)),
      ),
    );

    return Scaffold(
      body: ReorderableBuilder(
        children: generatedChildren,
        scrollController: _scrollController,
        onReorder: (List<OrderUpdateEntity> orderUpdateEntities) {
          for (final orderUpdateEntity in orderUpdateEntities) {
            final fruit = _fruits.removeAt(orderUpdateEntity.oldIndex);
            _fruits.insert(orderUpdateEntity.newIndex, fruit);
          }
        },
        builder: (children) {
          return GridView(
            key: _gridViewKey,
            controller: _scrollController,
            children: children,
            gridDelegate: const SliverGridDelegateWithFixedCrossAxisCount(
              crossAxisCount: 4,
              mainAxisSpacing: 4,
              crossAxisSpacing: 8,
            ),
          );
        },
      ),
    );
  }
}

class _Item extends StatelessWidget {
  const _Item({
    super.key,
    required List<String> fruits,
    required this.index
  }) : _fruits = fruits;

  final int index;
  final List<String> _fruits;

  @override
  Widget build(BuildContext context) {
    print("build index: $index");
    return Container(
      color: Colors.lightBlue,
      child: Text(
        _fruits.elementAt(index),
      ),
    );
  }
}
karvulf commented 1 year ago

Unfortunately I’m on vacation and it’s hard for me to test it but you should call setState in your onReorder function You could also update the package to the current pre release version but it’s still buggy @gmarizy

karvulf commented 1 year ago

If the rest works and it’s just that your item is getting rebuilt, that might be improved in the current pre release version. After my vacation (in 2 weeks) I can investigate the rebuilt @gmarizy

gmarizy commented 1 year ago

Enjoy your vacation then, I will wait no problem.

Also I tested with ^5.0.0-dev.7 and situation is even worse, there is more rebuild than with v4.