Fix DataTable example not being scrollable #131556

Chinmay-KB · 2023-07-29T17:28:46Z

Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.

Fixes the DataTable example not being scrollable in the docs
Added test for ensuring the DataTable is scrollable
Added the performance considerations when DataTable is wrapped in a SingleChildScrollView in docs.

Fixes #112755

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Chinmay-KB · 2023-07-29T17:29:02Z

The existing example has 10 items in DataTable, as all of them are already visible on the screen, writing a test for checking scrollability doesn't work, hence the items length was increased to 20. Does that work or should I do tester.binding.setSurfaceSize to a small enough size instead? @TahaTesser

navaronbracke · 2023-07-29T18:37:39Z

packages/flutter/lib/src/material/data_table.dart

@@ -357,6 +358,11 @@ class DataCell {
 /// [PaginatedDataTable] which automatically splits the data into
 /// multiple pages.
 ///
+/// ## Performance considerations when wrapping [DataTable] with [SingleChildScrollView]
+///
+/// Wrapping a [DataTable] with [SingleChildScrollView] is expensive as [SingleChildScrollView]


When flutter/packages#4536 lands, this could point to using TableView instead?

cc @Piinks

Yeah probably. :)

I would recommend adding some reference here to other options, not including TableView since that has not landed yet. If someone should not use a SingleChildScrollView because the table is very large, we should recommend PaginatedDataTable to split it up, or using a CustomScrollView.

TableView may not be relevant if someone only cares about scrolling in one direction, but we can add a reference to it when it is available.

examples/api/test/material/data_table/data_table.1_test.dart

TahaTesser · 2023-07-31T11:34:35Z

The existing example has 10 items in DataTable, as all of them are already visible on the screen, writing a test for checking scrollability doesn't work, hence the items length was increased to 20. Does that work or should I do tester.binding.setSurfaceSize to a small enough size instead? @TahaTesser

20 items sound good to me. 👍

packages/flutter/lib/src/material/data_table.dart

Chinmay-KB · 2023-08-01T10:35:42Z

Anything else I need to do to ready this PR @TahaTesser

examples/api/test/material/data_table/data_table.1_test.dart

TahaTesser · 2023-08-01T15:49:14Z

@Chinmay-KB

The class string for this newly added test class needs to be removed from _knownMissingTests set in dev/bots/check_code_samples.dart

flutter/dev/bots/check_code_samples.dart

Line 295 in bd64bf5

'examples/api/test/material/data_table/data_table.1_test.dart',

TahaTesser · 2023-08-03T07:42:09Z

packages/flutter/lib/src/material/data_table.dart

@@ -357,6 +357,11 @@ class DataCell {
 /// [PaginatedDataTable] which automatically splits the data into
 /// multiple pages.
 ///
+/// ## Performance considerations when wrapping [DataTable] with [SingleChildScrollView]


@Piinks Do you have any suggestions for this section?

Just the comment below, we should provide an alternative recommendation. :)

So, suggest using PaginatedDataTable wherever possible?
Or is there a better alternative

we should recommend PaginatedDataTable to split it up, or using a CustomScrollView.

I think those two would be the best recommendation for now. :)

Now that TableRow is merged to master, should that be added as a recommendation?

Let's just add the previously mentioned recommendations for now so we can land this PR. We'll likely do a sweep overall to see where we should be mentioning the TableView.

TahaTesser · 2023-09-07T08:06:34Z

packages/flutter/lib/src/material/data_table.dart

+  double minIntrinsicWidth(Iterable<RenderBox> cells, double containerWidth) =>
+      throw UnimplementedError();


Looks like you've made some unintended format changes.

Please read https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#formatting

Please revert the format changes

examples/api/lib/material/data_table/data_table.1.dart

Piinks

Resolved

MitchellGoodwin

LGTM!

auto-submit · 2023-09-11T23:50:12Z

auto label is removed for flutter/flutter/131556, due to This PR has not met approval requirements for merging. Changes were requested by {TahaTesser}, please make the needed changes and resubmit this PR.
You are not a member of flutter-hackers and need 1 more review(s) in order to merge this PR.

Merge guidelines: You need at least one approved review if you are already part of flutter-hackers or two member reviews if you are not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

Piinks · 2023-09-11T23:55:46Z

The autosubmit bot is wrong here, filed #134477
Merging manually.

…#131556)

flutter/flutter@219efce...4e7a07a 2023-09-12 katelovett@google.com Remove chip tooltip deprecations (flutter/flutter#134486) 2023-09-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from 8a8ddaeecf8a to d4698c65aa8d (2 revisions) (flutter/flutter#134553) 2023-09-12 katelovett@google.com Remove deprecated TextSelectionOverlay.fadeDuration (flutter/flutter#134485) 2023-09-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from 25be03186d82 to 8a8ddaeecf8a (1 revision) (flutter/flutter#134545) 2023-09-12 engine-flutter-autoroll@skia.org Roll Packages from ef0c65e to e04ba88 (5 revisions) (flutter/flutter#134546) 2023-09-12 47866232+chunhtai@users.noreply.github.com Revert "Adds a parent scope TraversalEdgeBehavior and fixes modal rouâ�¦ (flutter/flutter#134550) 2023-09-12 fluttergithubbot@gmail.com Marks Windows_android channels_integration_test_win to be unflaky (flutter/flutter#133129) 2023-09-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4091167e402d to 25be03186d82 (1 revision) (flutter/flutter#134536) 2023-09-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from 54175da7ba90 to 4091167e402d (1 revision) (flutter/flutter#134532) 2023-09-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from 85f3f02e5c37 to 54175da7ba90 (2 revisions) (flutter/flutter#134531) 2023-09-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1369ab35aaa7 to 85f3f02e5c37 (1 revision) (flutter/flutter#134519) 2023-09-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2f1efe5967f3 to 1369ab35aaa7 (1 revision) (flutter/flutter#134512) 2023-09-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4d8265cbc133 to 2f1efe5967f3 (3 revisions) (flutter/flutter#134511) 2023-09-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from ee7215553deb to 4d8265cbc133 (1 revision) (flutter/flutter#134506) 2023-09-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6ddee4423d2c to ee7215553deb (2 revisions) (flutter/flutter#134505) 2023-09-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from a98348946d61 to 6ddee4423d2c (1 revision) (flutter/flutter#134490) 2023-09-12 xubaolin@oppo.com [New feature] Allowing the `ListView` slivers to have different extents while still having scrolling performance (flutter/flutter#131393) 2023-09-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from 8389ad914b21 to a98348946d61 (4 revisions) (flutter/flutter#134487) 2023-09-12 kouhei.seino@woven-planet.global ScaleGestureRecognizer: make pointerCount public (flutter/flutter#127310) 2023-09-11 chinmay@blend.to Fix DataTable example not being scrollable (flutter/flutter#131556) 2023-09-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 06696e768c1b to 8389ad914b21 (2 revisions) (flutter/flutter#134469) 2023-09-11 christopherfujino@gmail.com [flutter_tools] disallow -O0 for flutter build web (flutter/flutter#134185) 2023-09-11 sokolovskyi.konstantin@gmail.com Cover focus tests with leak tracking (flutter/flutter#134457) 2023-09-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0774ddcda9b0 to 06696e768c1b (4 revisions) (flutter/flutter#134462) 2023-09-11 sokolovskyi.konstantin@gmail.com Fix memory leak in RenderAnimatedSize (flutter/flutter#133653) 2023-09-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from e42fd367adcb to 0774ddcda9b0 (1 revision) (flutter/flutter#134447) 2023-09-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from d593002b7159 to e42fd367adcb (1 revision) (flutter/flutter#134444) 2023-09-11 engine-flutter-autoroll@skia.org Roll Packages from aaae5ef to ef0c65e (6 revisions) (flutter/flutter#134445) 2023-09-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2b5961cbc40d to d593002b7159 (3 revisions) (flutter/flutter#134439) 2023-09-11 polinach@google.com Mark leak: instances of OpacityLayer, created by _RenderChip, should be disposed. (flutter/flutter#134395) 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 Please CC camillesimon@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…#131556)

Chinmay-KB and others added 2 commits July 29, 2023 16:54

test: add test for data table example

a6078bc

add performance consideration doc for data table

db6baab

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos labels Jul 29, 2023

fix analyzer issue

2a8a1d7

navaronbracke reviewed Jul 29, 2023

View reviewed changes

TahaTesser requested changes Jul 31, 2023

View reviewed changes

examples/api/test/material/data_table/data_table.1_test.dart Outdated Show resolved Hide resolved

TahaTesser requested changes Jul 31, 2023

View reviewed changes

packages/flutter/lib/src/material/data_table.dart Outdated Show resolved Hide resolved

TahaTesser and others added 3 commits July 31, 2023 14:36

Update examples/api/test/material/data_table/data_table.1_test.dart

2ea254c

Update packages/flutter/lib/src/material/data_table.dart

7edb09a

Merge branch 'master' into issue-112755

aeacdd0

TahaTesser requested changes Aug 1, 2023

View reviewed changes

examples/api/test/material/data_table/data_table.1_test.dart Show resolved Hide resolved

Update examples/api/test/material/data_table/data_table.1_test.dart

b0b19e5

Chinmay-KB and others added 2 commits August 3, 2023 08:36

Merge branch 'master' into issue-112755

27c0fde

remove test path from _knownMissingTests

2b2e9ef

Chinmay-KB requested a review from TahaTesser August 3, 2023 03:43

TahaTesser requested a review from Piinks August 3, 2023 07:41

TahaTesser reviewed Aug 3, 2023

View reviewed changes

This comment was marked as spam.

Sign in to view

TahaTesser mentioned this pull request Aug 16, 2023

DataTable example is scrollable #132563

Closed

docs: add more suggestion

0f191a1

Chinmay-KB requested a review from TahaTesser September 1, 2023 21:30

Merge remote-tracking branch 'origin/master' into issue-112755

5293a80

TahaTesser requested changes Sep 7, 2023

View reviewed changes

Chinmay-KB added 2 commits September 8, 2023 11:40

further fixes for formatting

813a00f

fix: eol

7d336a9

Chinmay-KB requested a review from TahaTesser September 8, 2023 06:16

TahaTesser previously requested changes Sep 8, 2023

View reviewed changes

examples/api/lib/material/data_table/data_table.1.dart Outdated Show resolved Hide resolved

Update examples/api/lib/material/data_table/data_table.1.dart

bc7d717

Piinks approved these changes Sep 11, 2023

View reviewed changes

MitchellGoodwin approved these changes Sep 11, 2023

View reviewed changes

Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 11, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 11, 2023

Piinks mentioned this pull request Sep 11, 2023

Autosubmit bot will not merge, claiming false negatives #134477

Open

Piinks merged commit 4a3ab68 into flutter:master Sep 11, 2023
131 of 132 checks passed

Chinmay-KB deleted the issue-112755 branch September 12, 2023 14:16

engine-flutter-autoroll mentioned this pull request Sep 12, 2023

Roll Flutter from 219efce7f16d to 127c76b91bef (22 revisions) flutter/packages#4905

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2023

4a3ab6828 Fix DataTable example not being scrollable (flutter/flutter…

f429ce7

…#131556)

engine-flutter-autoroll mentioned this pull request Sep 12, 2023

Roll Flutter from 219efce7f16d to bdd96032a053 (24 revisions) flutter/packages#4906

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2023

4a3ab6828 Fix DataTable example not being scrollable (flutter/flutter…

6ca26de

…#131556)

engine-flutter-autoroll mentioned this pull request Sep 12, 2023

Roll Flutter from 219efce7f16d to 5900c4baa751 (25 revisions) flutter/packages#4907

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2023

4a3ab6828 Fix DataTable example not being scrollable (flutter/flutter…

9608ed3

…#131556)

engine-flutter-autoroll mentioned this pull request Sep 12, 2023

Roll Flutter from 219efce7f16d to f9aef4f21e31 (26 revisions) flutter/packages#4908

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2023

4a3ab6828 Fix DataTable example not being scrollable (flutter/flutter…

411ad22

…#131556)

engine-flutter-autoroll mentioned this pull request Sep 12, 2023

Roll Flutter from 219efce7f16d to 90ae98f57642 (28 revisions) flutter/packages#4909

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2023

4a3ab6828 Fix DataTable example not being scrollable (flutter/flutter…

ba55f5f

…#131556)

engine-flutter-autoroll mentioned this pull request Sep 12, 2023

Roll Flutter from 219efce7f16d to 4e7a07af882c (30 revisions) flutter/packages#4910

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2023

4a3ab6828 Fix DataTable example not being scrollable (flutter/flutter…

a755b35

…#131556)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2023

4a3ab6828 Fix DataTable example not being scrollable (flutter/flutter…

1941a44

…#131556)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix DataTable example not being scrollable #131556

Fix DataTable example not being scrollable #131556

Chinmay-KB commented Jul 29, 2023

Chinmay-KB commented Jul 29, 2023

navaronbracke Jul 29, 2023

Piinks Aug 8, 2023

TahaTesser commented Jul 31, 2023

Chinmay-KB commented Aug 1, 2023

TahaTesser commented Aug 1, 2023 •

edited

TahaTesser Aug 3, 2023

Piinks Aug 10, 2023

Chinmay-KB Aug 12, 2023

Piinks Aug 15, 2023

Chinmay-KB Aug 24, 2023

Piinks Aug 24, 2023

This comment was marked as spam.

TahaTesser Sep 7, 2023

Chinmay-KB Sep 8, 2023

Piinks left a comment

MitchellGoodwin left a comment

auto-submit bot commented Sep 11, 2023

Piinks commented Sep 11, 2023

		double minIntrinsicWidth(Iterable<RenderBox> cells, double containerWidth) =>
		throw UnimplementedError();

Fix DataTable example not being scrollable #131556

Fix DataTable example not being scrollable #131556

Conversation

Chinmay-KB commented Jul 29, 2023

Pre-launch Checklist

Chinmay-KB commented Jul 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TahaTesser commented Jul 31, 2023

Chinmay-KB commented Aug 1, 2023

TahaTesser commented Aug 1, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as spam.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Piinks left a comment

Choose a reason for hiding this comment

MitchellGoodwin left a comment

Choose a reason for hiding this comment

auto-submit bot commented Sep 11, 2023

Piinks commented Sep 11, 2023

TahaTesser commented Aug 1, 2023 •

edited