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

ReorderableGridView with shrinkWrap expands vertically when reordering, if last row is 'full' #15

Closed
olof-dev opened this issue Jun 19, 2022 · 7 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@olof-dev
Copy link
Contributor

If a ReorderableGridView.extent has shrinkWrap set to true, and the last row is full of items, then an extra row is added to the grid view when reordering is started:

reorderable_shrinkwrap_bug.mp4

This does not happen if the last row is not full. I would expect it not to happen in general.

Thanks again!

import 'package:flutter/material.dart';
import 'package:reorderable_grid/reorderable_grid.dart';

void main() => runApp(const MyApp());

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

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

class _MyAppState extends State<MyApp> {
  final items = List<int>.generate(3, (index) => index);

  void _onReorder(int oldIndex, int newIndex) {
    setState(() {
      final item = items.removeAt(oldIndex);
      items.insert(newIndex, item);
    });
  }

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      debugShowCheckedModeBanner: false,
      home: Scaffold(
        body: Column(
          children: [
            Container(
              color: Colors.blue,
              child: ReorderableGridView.extent(
                shrinkWrap: true,
                maxCrossAxisExtent: 250,
                onReorder: _onReorder,
                childAspectRatio: 1,
                children: items.map((item) {
                  return Card(
                    key: ValueKey(item),
                    child: Center(
                      child: Text(item.toString()),
                    ),
                  );
                }).toList(),
              ),
            ),
            const Text("Below"),
          ],
        ),
      ),
    );
  }
}
@casvanluijtelaar
Copy link
Owner

I'm aware of this, it is somewhat of a "hack" to make extend work properly. there might be a better way to do this but I haven't found one yet.

 @override
  Widget build(BuildContext context) {
    assert(debugCheckHasOverlay(context));
    final SliverChildBuilderDelegate childrenDelegate =
        SliverChildBuilderDelegate(
      _itemBuilder,
      // When dragging, the dragged item is still in the grid but has been replaced
      // by a zero height SizedBox, so that the gap can move around. To make the
      // grid extent stable we add a dummy entry to the end.
      childCount: widget.itemCount + (_dragInfo != null ? 1 : 0),
    );
    return SliverGrid(
      delegate: childrenDelegate,
      gridDelegate: widget.gridDelegate,
    );
  }

@olof-dev
Copy link
Contributor Author

@casvanluijtelaar I'm not sure precisely how the logic in the package works, but I don't see why the following wouldn't work without side effects:

  1. The grid always has a constant number of items.
  2. Items in the grid are always placed at the 'correct index', but can be rendered at a different position using Transform.translate.
  3. When an item is dragged, the widget at its original index is replaced by a SizedBox of exactly its size, and the original item widget follows the drag in an Overlay.
  4. TheSizedBox in the dragged item's index tracks the drag, but snapped to the grid, by a Transform.translate on it.
  5. Each grid item with an index between the dragged item's original index and its current 'hovered index' (insertIndex) has an offset applied to its painting using Transform.translate.

Is this similar to how the current code works? Maybe the current code can be changed to this with little effort? I tried updating the parts of it that I could decipher to follow this logic, but it has some effects that I couldn't understand, given that I don't understand the overall design.

@Enigma56
Copy link

Enigma56 commented Jan 7, 2023

Out of curiosity, has a fix for this been found yet? I have been running into the same issue in my project and NEED a solution. I am willing to work on one myself as well if one has not yet been found.

@olof-dev
Copy link
Contributor Author

olof-dev commented Jan 7, 2023

I am not aware of an existing fix. I think the strategy in #15 (comment) should work, but I didn't have time to figure out the overall design of the package.

@casvanluijtelaar
Copy link
Owner

I haven't looked into it much, it could potentially be fixed with some conditional logic ontop of the current implementation. for example only adding a new row when the drag position is on the last item etc

@olof-dev
Copy link
Contributor Author

I believe #17 fixes this. Please test it and let me know!

@casvanluijtelaar
Copy link
Owner

fixed in 1.0.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants