caduandrade / multi_split_view

Provides horizontal or vertical multiple split view for Flutter.
MIT License
152 stars 29 forks source link

Having children with `GlobalKey` crashes since 2.0.1 #58

Closed oO0oO0oO0o0o00 closed 5 months ago

oO0oO0oO0o0o00 commented 5 months ago

The bug was introduced in deb0f87 which fixes local keys not working. When global keys are used, there's no need to pass them to wrapper views since they are global.

Reproduce:

  1. Create empty app project (optionally).
  2. Replace main.dart with coed below:
    
    import 'package:flutter/material.dart';
    import 'package:multi_split_view/multi_split_view.dart';

void main() { runApp(const MainApp()); }

class MainApp extends StatelessWidget { const MainApp({super.key});

@override Widget build(BuildContext context) { return const MaterialApp( home: Scaffold( body: S(), ), ); } }

class S extends StatefulWidget { const S({super.key}); @override State createState() => SS(); }

class SS extends State { final k = GlobalKey();

@override Widget build(BuildContext context) { return MultiSplitView(children: [const Text('Hello World!'), TextField(key: k)]); } }

3. Add dependency:
```yaml
dependencies:
  flutter:
    sdk: flutter
  multi_split_view:
    git:
        url: https://github.com/caduandrade/multi_split_view.git
        ref: deb0f87d5ac86623a15b19bcf7a65f9d74edafda
  1. Run and observe the crash.
  2. Change the ref to its parent and rerun without problem.

Potential fix: In lib/src/multi_split_view.dart:655

- key: child.key,
+ key: child.key is GlobalKey ? null : child.key,
caduandrade commented 5 months ago

Hi @oO0oO0oO0o0o00! What username is that? :smile:

This problem will be resolved by #54 but it is still fragile due to the resolution of inconsistencies and the creation of areas independent of the list of children. In the new solution, the key is unique per Area but it is still possible to remove a child (the first for example) from the list of children without removing the area from the controller. At this point, inconsistency resolution will remove the last unusable area of the controller. In this case, the new first child (second before removing) will use the key from the first area that was not removed by the developer.

I think the only way to guarantee the correct use of the keys and not have a problem with StatefulWidget as children would be to no longer allow passing the list of children in the MultiSplitView. Maybe you must always define it using just the controller and each area with some WidgetBuilder to know how to create your widget.

Thus, when removing an area, you will remove the Widget and the key together.

API usage will change slightly.

caduandrade commented 5 months ago

Done (version 3.0.0)