Skip to content

Conversation

@TahaTesser
Copy link
Member

Fixes ListTile Crashes When Width Is Reduced with Leading or Trailing Widgets

Pre-launch Checklist

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

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Jan 3, 2025
@nate-thegrate nate-thegrate self-requested a review January 8, 2025 05:36
Copy link
Contributor

@nate-thegrate nate-thegrate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the way ListTile works currently makes the most sense; much like a flex overflow, it should ideally be up to the developer to prevent scenarios where list tiles don't have enough space.

I'm thinking it might be best to refactor those assert messages into FlutterErrors so that people don't file bug reports in the future.

@TahaTesser TahaTesser force-pushed the fix_list_tile_leading_trailng_size_error_message branch from 6ac1b87 to bc6a2df Compare January 8, 2025 09:52
@Piinks Piinks requested a review from victorsanni January 8, 2025 18:26
Copy link
Contributor

@nate-thegrate nate-thegrate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update! It's unfortunate to see all those hanging checks, hopefully that can be resolved soon 🤞

@TahaTesser TahaTesser force-pushed the fix_list_tile_leading_trailng_size_error_message branch from e91a223 to aebd1c8 Compare January 10, 2025 08:51
Copy link
Contributor

@nate-thegrate nate-thegrate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

await tester.pumpWidget(buildListTile(leading: const SizedBox(width: 61)));

// Error message cannot be tested as there too many errors thrown.
expect(tester.takeException(), isNotNull);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test throws errors but still passes on master. Is this expected?

Copy link
Member Author

@TahaTesser TahaTesser Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this type of runtime error throws in the logs but we're catching it with tester.takeException() and expecting the test to pass, regardless of the logs. Some of the existing widgets do this as well.

@TahaTesser TahaTesser force-pushed the fix_list_tile_leading_trailng_size_error_message branch from c3b11af to 55b166c Compare January 13, 2025 10:36
@TahaTesser TahaTesser added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 13, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Jan 13, 2025
Merged via the queue into flutter:master with commit cef35e7 Jan 13, 2025
102 of 103 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 13, 2025
@TahaTesser TahaTesser deleted the fix_list_tile_leading_trailng_size_error_message branch January 13, 2025 15:10
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 13, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jan 13, 2025
flutter/flutter@864d4f5...72db8f6

2025-01-13 [email protected] Roll Packages from 6554751 to 3c3bc68 (16 revisions) (flutter/flutter#161515)
2025-01-13 [email protected] Update error message for when leading/trailing width exceeds `ListTile` width and add missing test (flutter/flutter#161091)
2025-01-13 [email protected] Deprecate unused `ButtonStyleButton.iconAlignment` property (flutter/flutter#160023)
2025-01-12 [email protected] Provide monitor information. (flutter/flutter#161359)
2025-01-11 [email protected] [android_engine_test] Remove background/foreground from surface texture trampoline test. (flutter/flutter#161441)
2025-01-11 [email protected] Remove some miscellaneous references to Cirrus. (flutter/flutter#161390)
2025-01-11 [email protected] add semantics role and tab (flutter/flutter#161260)
2025-01-10 [email protected] CupertinoSheetRoute (flutter/flutter#157568)
2025-01-10 [email protected] Update `TextEditingController.text` documentation to recommend against using it in production code (flutter/flutter#157769)
2025-01-10 [email protected] ð��� [tool] Add a wirelessly connected device name as `displayName` (flutter/flutter#160497)
2025-01-10 [email protected] FixForward: method was renamed (flutter/flutter#161431)

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] 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://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
maheshj01 pushed a commit to maheshj01/flutter that referenced this pull request Jan 15, 2025
…e` width and add missing test (flutter#161091)

Fixes [ListTile Crashes When Width Is Reduced with Leading or Trailing
Widgets](flutter#159380)

## Pre-launch Checklist

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

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025
androidseb pushed a commit to androidseb/packages that referenced this pull request Jun 8, 2025
flutter/flutter@864d4f5...72db8f6

2025-01-13 [email protected] Roll Packages from 6554751 to 3c3bc68 (16 revisions) (flutter/flutter#161515)
2025-01-13 [email protected] Update error message for when leading/trailing width exceeds `ListTile` width and add missing test (flutter/flutter#161091)
2025-01-13 [email protected] Deprecate unused `ButtonStyleButton.iconAlignment` property (flutter/flutter#160023)
2025-01-12 [email protected] Provide monitor information. (flutter/flutter#161359)
2025-01-11 [email protected] [android_engine_test] Remove background/foreground from surface texture trampoline test. (flutter/flutter#161441)
2025-01-11 [email protected] Remove some miscellaneous references to Cirrus. (flutter/flutter#161390)
2025-01-11 [email protected] add semantics role and tab (flutter/flutter#161260)
2025-01-10 [email protected] CupertinoSheetRoute (flutter/flutter#157568)
2025-01-10 [email protected] Update `TextEditingController.text` documentation to recommend against using it in production code (flutter/flutter#157769)
2025-01-10 [email protected] ð��� [tool] Add a wirelessly connected device name as `displayName` (flutter/flutter#160497)
2025-01-10 [email protected] FixForward: method was renamed (flutter/flutter#161431)

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] 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://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
FMorschel pushed a commit to FMorschel/packages that referenced this pull request Jun 9, 2025
flutter/flutter@864d4f5...72db8f6

2025-01-13 [email protected] Roll Packages from 6554751 to 3c3bc68 (16 revisions) (flutter/flutter#161515)
2025-01-13 [email protected] Update error message for when leading/trailing width exceeds `ListTile` width and add missing test (flutter/flutter#161091)
2025-01-13 [email protected] Deprecate unused `ButtonStyleButton.iconAlignment` property (flutter/flutter#160023)
2025-01-12 [email protected] Provide monitor information. (flutter/flutter#161359)
2025-01-11 [email protected] [android_engine_test] Remove background/foreground from surface texture trampoline test. (flutter/flutter#161441)
2025-01-11 [email protected] Remove some miscellaneous references to Cirrus. (flutter/flutter#161390)
2025-01-11 [email protected] add semantics role and tab (flutter/flutter#161260)
2025-01-10 [email protected] CupertinoSheetRoute (flutter/flutter#157568)
2025-01-10 [email protected] Update `TextEditingController.text` documentation to recommend against using it in production code (flutter/flutter#157769)
2025-01-10 [email protected] ð��� [tool] Add a wirelessly connected device name as `displayName` (flutter/flutter#160497)
2025-01-10 [email protected] FixForward: method was renamed (flutter/flutter#161431)

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] 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://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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ListTile Crashes When Width Is Reduced with Leading or Trailing Widgets

3 participants