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

 ///
+/// ## 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

 /// [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.

…31556)

flutter/flutter@219efce...4e7a07a 2023-09-12 [email protected] Remove chip tooltip deprecations (flutter/flutter#134486) 2023-09-12 [email protected] Roll Flutter Engine from 8a8ddaeecf8a to d4698c65aa8d (2 revisions) (flutter/flutter#134553) 2023-09-12 [email protected] Remove deprecated TextSelectionOverlay.fadeDuration (flutter/flutter#134485) 2023-09-12 [email protected] Roll Flutter Engine from 25be03186d82 to 8a8ddaeecf8a (1 revision) (flutter/flutter#134545) 2023-09-12 [email protected] Roll Packages from ef0c65e to e04ba88 (5 revisions) (flutter/flutter#134546) 2023-09-12 [email protected] Revert "Adds a parent scope TraversalEdgeBehavior and fixes modal rouâ�¦ (flutter/flutter#134550) 2023-09-12 [email protected] Marks Windows_android channels_integration_test_win to be unflaky (flutter/flutter#133129) 2023-09-12 [email protected] Roll Flutter Engine from 4091167e402d to 25be03186d82 (1 revision) (flutter/flutter#134536) 2023-09-12 [email protected] Roll Flutter Engine from 54175da7ba90 to 4091167e402d (1 revision) (flutter/flutter#134532) 2023-09-12 [email protected] Roll Flutter Engine from 85f3f02e5c37 to 54175da7ba90 (2 revisions) (flutter/flutter#134531) 2023-09-12 [email protected] Roll Flutter Engine from 1369ab35aaa7 to 85f3f02e5c37 (1 revision) (flutter/flutter#134519) 2023-09-12 [email protected] Roll Flutter Engine from 2f1efe5967f3 to 1369ab35aaa7 (1 revision) (flutter/flutter#134512) 2023-09-12 [email protected] Roll Flutter Engine from 4d8265cbc133 to 2f1efe5967f3 (3 revisions) (flutter/flutter#134511) 2023-09-12 [email protected] Roll Flutter Engine from ee7215553deb to 4d8265cbc133 (1 revision) (flutter/flutter#134506) 2023-09-12 [email protected] Roll Flutter Engine from 6ddee4423d2c to ee7215553deb (2 revisions) (flutter/flutter#134505) 2023-09-12 [email protected] Roll Flutter Engine from a98348946d61 to 6ddee4423d2c (1 revision) (flutter/flutter#134490) 2023-09-12 [email protected] [New feature] Allowing the `ListView` slivers to have different extents while still having scrolling performance (flutter/flutter#131393) 2023-09-12 [email protected] Roll Flutter Engine from 8389ad914b21 to a98348946d61 (4 revisions) (flutter/flutter#134487) 2023-09-12 [email protected] ScaleGestureRecognizer: make pointerCount public (flutter/flutter#127310) 2023-09-11 [email protected] Fix DataTable example not being scrollable (flutter/flutter#131556) 2023-09-11 [email protected] Roll Flutter Engine from 06696e768c1b to 8389ad914b21 (2 revisions) (flutter/flutter#134469) 2023-09-11 [email protected] [flutter_tools] disallow -O0 for flutter build web (flutter/flutter#134185) 2023-09-11 [email protected] Cover focus tests with leak tracking (flutter/flutter#134457) 2023-09-11 [email protected] Roll Flutter Engine from 0774ddcda9b0 to 06696e768c1b (4 revisions) (flutter/flutter#134462) 2023-09-11 [email protected] Fix memory leak in RenderAnimatedSize (flutter/flutter#133653) 2023-09-11 [email protected] Roll Flutter Engine from e42fd367adcb to 0774ddcda9b0 (1 revision) (flutter/flutter#134447) 2023-09-11 [email protected] Roll Flutter Engine from d593002b7159 to e42fd367adcb (1 revision) (flutter/flutter#134444) 2023-09-11 [email protected] Roll Packages from aaae5ef to ef0c65e (6 revisions) (flutter/flutter#134445) 2023-09-11 [email protected] Roll Flutter Engine from 2b5961cbc40d to d593002b7159 (3 revisions) (flutter/flutter#134439) 2023-09-11 [email protected] 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 [email protected],[email protected],[email protected] 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

…31556)

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

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

Closed

Piinks merged commit 4a3ab68 into flutter:master Sep 11, 2023

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

4a3ab68 Fix DataTable example not being scrollable (flutter/flutter#1…

f429ce7

…31556)

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

4a3ab68 Fix DataTable example not being scrollable (flutter/flutter#1…

6ca26de

…31556)

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

4a3ab68 Fix DataTable example not being scrollable (flutter/flutter#1…

9608ed3

…31556)

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

4a3ab68 Fix DataTable example not being scrollable (flutter/flutter#1…

411ad22

…31556)

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

4a3ab68 Fix DataTable example not being scrollable (flutter/flutter#1…

ba55f5f

…31556)

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

4a3ab68 Fix DataTable example not being scrollable (flutter/flutter#1…

a755b35

…31556)

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

4a3ab68 Fix DataTable example not being scrollable (flutter/flutter#1…

1941a44

…31556)

darshankawar mentioned this pull request May 29, 2024

Provides a high-performance example of the CustomScrollView using the dataTable #149151

Closed

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

Fix DataTable example not being scrollable #131556

Fix DataTable example not being scrollable #131556

Uh oh!

Conversation

Chinmay-KB commented Jul 29, 2023

Pre-launch Checklist

Uh oh!

Chinmay-KB commented Jul 29, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TahaTesser commented Jul 31, 2023

Uh oh!

Uh oh!

Chinmay-KB commented Aug 1, 2023

Uh oh!

Uh oh!

TahaTesser commented Aug 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

MitchellGoodwin left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Sep 11, 2023

Uh oh!

Piinks commented Sep 11, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

TahaTesser commented Aug 1, 2023 •

edited

Loading