maxim-saplin / data_table_2

In-place substitute for Flutter's DataTable and PaginatedDataTable with fixed/sticky header and extra features
https://pub.dev/packages/data_table_2
BSD 3-Clause "New" or "Revised" License
205 stars 144 forks source link

PageStorage compatibility #155

Open jpnurmi opened 2 years ago

jpnurmi commented 2 years ago

Would it be possible to pass keepScrollOffset: false for the internal "synced" ScrollControllers to avoid conflicts with PageStorage when saving and restoring scrolling positions?

Here's a dummy example to illustrate the issue. Tap a table row and "go back" to navigate away and back. The table should restore its scrolling positions.

keep-scroll-offset.webm

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

void main() {
  runApp(
    MaterialApp(
      initialRoute: '/table',
      routes: {
        '/table': (_) => const TablePage(),
        '/other': (_) => const OtherPage(),
      },
    ),
  );
}

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

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: ElevatedButton(
          onPressed: () => Navigator.of(context).pushReplacementNamed('/table'),
          child: const Text('Go back'),
        ),
      ),
    );
  }
}

final restorationBucket = PageStorageBucket();

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

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: PageStorage(
        bucket: restorationBucket,
        child: DataTable2(
          key: const PageStorageKey('example'),
          columnSpacing: 12,
          horizontalMargin: 12,
          minWidth: 600,
          columns: const [
            DataColumn2(
              label: Text('Column A'),
              size: ColumnSize.L,
            ),
            DataColumn(
              label: Text('Column B'),
            ),
            DataColumn(
              label: Text('Column C'),
            ),
            DataColumn(
              label: Text('Column D'),
            ),
            DataColumn(
              label: Text('Column NUMBERS'),
              numeric: true,
            ),
          ],
          rows: List<DataRow>.generate(
            100,
            (index) => DataRow2(
              cells: [
                DataCell(Text('A' * (10 - index % 10))),
                DataCell(Text('B' * (10 - (index + 5) % 10))),
                DataCell(Text('C' * (15 - (index + 5) % 10))),
                DataCell(Text('D' * (15 - (index + 10) % 10))),
                DataCell(Text(((index + 0.1) * 25.4).toString()))
              ],
              onTap: () => Navigator.pushReplacementNamed(context, '/other'),
            ),
          ),
        ),
      ),
    );
  }
}
maxim-saplin commented 2 years ago

Changing that field is not a problem, though I'm not sure of side effects of such a change, that would require thorough testing. If you can submit a PR, add some sample test reproducing the case and add widget tests - I'm happy to review and merge it