devtools icon indicating copy to clipboard operation
devtools copied to clipboard

Flat_table does not sort columns correctly.

Open polina-c opened this issue 3 years ago • 2 comments

Commit: 2db25e0b52e196c7aab633143599e294cc29627d

Repro:

  • run flutter run -t test/scenes/memory/default.stager_app.dart -d macos
  • switch between single and diff table for class details: https://user-images.githubusercontent.com/12115586/194680564-c67cd06d-1477-4274-af65-8afaa603fb55.mov

Code:

class _RetainingPathTable extends StatelessWidget {
  _RetainingPathTable({
    Key? key,
    required this.entries,
    required this.selection,
    required this.isDiff,
  }) : super(key: key);

  final List<StatsByPathEntry> entries;
  final ValueNotifier<StatsByPathEntry?> selection;
  final bool isDiff;

  late final _shallowSizeColumn = _ShallowSizeColumn(isDiff);
  late final List<ColumnData<StatsByPathEntry>> _columns =
      <ColumnData<StatsByPathEntry>>[
    _RetainingPathColumn(),
    _InstanceColumn(isDiff),
    _shallowSizeColumn,
    _RetainedSizeColumn(isDiff),
  ];

  @override
  Widget build(BuildContext context) {
    return FlatTable<StatsByPathEntry>(
      dataKey: 'RetainingPathTable-$isDiff',
      columns: _columns,
      data: entries,
      keyFactory: (e) => Key(e.key.asLongString()),
      selectionNotifier: selection,
      defaultSortColumn: _shallowSizeColumn,
      defaultSortDirection: SortDirection.descending,
    );
  }
}

Fix:

class _RetainingPathTableColumns {
  _RetainingPathTableColumns(this.isDiff);

  final bool isDiff;
  late final shallowSizeColumn = _ShallowSizeColumn(isDiff);
  late final columnList = <ColumnData<StatsByPathEntry>>[
    _RetainingPathColumn(),
    _InstanceColumn(isDiff),
    shallowSizeColumn,
    _RetainedSizeColumn(isDiff),
  ];
}

class _RetainingPathTable extends StatelessWidget {
  const _RetainingPathTable({
    Key? key,
    required this.entries,
    required this.selection,
    required this.isDiff,
  }) : super(key: key);

  final List<StatsByPathEntry> entries;
  final ValueNotifier<StatsByPathEntry?> selection;
  final bool isDiff;

  static final _columns = <String, _RetainingPathTableColumns>{};
  static _RetainingPathTableColumns _obtainColumns(
          String dataKey, bool isDiff) =>
      _columns.putIfAbsent(dataKey, () => _RetainingPathTableColumns(isDiff));

  @override
  Widget build(BuildContext context) {
    final dataKey = 'RetainingPathTable-${identityHashCode(entries)}';
    final columns = _obtainColumns(dataKey, isDiff);
    return FlatTable<StatsByPathEntry>(
      dataKey: dataKey,
      columns: columns.columnList,
      data: entries,
      keyFactory: (e) => Key(e.key.asLongString()),
      selectionNotifier: selection,
      defaultSortColumn: columns.shallowSizeColumn,
      defaultSortDirection: SortDirection.descending,
    );
  }
}

polina-c avatar Oct 08 '22 01:10 polina-c

@kenzieschmoll

polina-c avatar Oct 08 '22 01:10 polina-c

Would it be more traditional and convenient, instead of asking for dataKey, to accept a TableController in constructor? Like for TextFileld, the controller would allow both widget and whoever has access to the controller, to modify the widget's state. And, it will be up to developer, what parts of the state to share with other instances of the table.

The controller can contain:

  • scroll controller
  • selection
  • sorting (sort column should be specified by index, not by instance?)

How does it sound?

polina-c avatar Oct 08 '22 19:10 polina-c