[two_dimensional_scrollables] Infinite TableView #6411
Conversation
Piinks
added
the
p: two_dimensional_scrollables
packages/two_dimensional_scrollables/lib/src/table_view/table_delegate.dart
Outdated
Show resolved
Hide resolved
| return switch (maxXIndex) { | ||
| null => maxXIndex, | ||
| _ => maxXIndex! + 1, | ||
| }; |
There was a problem hiding this comment.
switch statements are cool and all, but IMO this could be more clearly expressed as:
| return switch (maxXIndex) { | |
| null => maxXIndex, | |
| _ => maxXIndex! + 1, | |
| }; | |
| return maxXIndex == null ? null : maxXIndex! + 1; |
(here and elsewhere where this pattern is used)
There was a problem hiding this comment.
Yeah, I may have gotten carried away here. 😜
| ), | ||
| ); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
There are a lot of test cases here, so maybe I missed it: Do you have a test case where it changes at what index columnBuilder/rowBuilder returns null? Let's say initially, columnBuilder/rowBuilder return null it index 10. Is it properly handled if later two rows are added and they now return null at index 12? Same if two rows are removed and they now return null at index 8?
There was a problem hiding this comment.
Oh excellent test cases!! I will add these. I suspect I do not handle this well currently... I'll check and see how ListView does this. I have an idea, but this problem is already solved there. 🙃
| int? _lastNonPinnedRow; | ||
| int? _lastNonPinnedColumn; | ||
|
|
||
| int? _columnNullTerminated; |
There was a problem hiding this comment.
for my own understanding (and maybe to clarify in the name): This is the index at which the columns are null terminated, right? The current name made it sound like this should be a boolean indicating whether the column builder has returned null yet.
There was a problem hiding this comment.
Correct! I am changing the name to indicate this better. Funny you imagined a bool - at one point it was!
| _lastNonPinnedColumn = null; | ||
| double startOfRegularColumn = 0; | ||
| double startOfPinnedColumn = 0; | ||
| // Updates the cached column metrics for the table. |
There was a problem hiding this comment.
To help future readers: maybe document the meaning of the arguments to this method here since they are not immediately clear.
packages/two_dimensional_scrollables/lib/src/table_view/table_delegate.dart
Outdated
Show resolved
Hide resolved
|
Ok, this is ready for another look, the diff is contained in these commits: https://github.com/flutter/packages/pull/6411/files/7ed44342c35a0292645a5b34905efa2339a7662b..7b4b3d6d79a9bfdde4330461fa49dca32588afc6 |
| // No columns laid out after column 5. | ||
| expect(find.text('R0:C5'), findsNothing); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Thanks for adding these additional test cases!
There was a problem hiding this comment.
It was a great catch! Thank you!
flutter/packages@51faaa1...d5aff19 2024-03-30 [email protected] Roll Flutter from 8528881 to d12ba5c (21 revisions) (flutter/packages#6440) 2024-03-29 [email protected] Roll Flutter from 89ea492 to 8528881 (11 revisions) (flutter/packages#6436) 2024-03-29 [email protected] [two_dimensional_scrollables] Infinite TableView (flutter/packages#6411) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
2 participants
Fixes flutter/flutter#131226
This adds support for infinite rows and columns in TableView.
When using a TableCellBuilderDelegate or the TableView.builder constructor, the row and column counts are no longer required. Omitting them will result in an infinite number or rows and/or columns.
When the row and column count are finite, the layout of the table is eagerly computed, while the children are actually laid out lazily. In the infinite TableView world, both the layout computation and the layout of children is lazy. The metrics for the table will only be computed for what is visible or in the cache extent, and will update the metrics as needed.
Similar to widgets like ListView, there is an option to null terminate the number of rows or columns in the table. The table's rows and/or columns will be infinite in number unless the rowBuilder or columnBuilder returns null, signifying the end. The ScrollPosition will reflect a maxScrollExtent of double.infinity unless the null terminating row or column has been reached, similar to other infinite-until-null-terminated widgets like ListView.
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.