Skip to content

Conversation

@ahmedsameha1
Copy link
Contributor

This is my attempt to handle #6537 for the CheckboxListTile UI control.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Aug 2, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This PR adds a regression test for a crash in CheckboxListTile when rendered in a zero-sized area. The test is a good addition, but the PR seems to be missing the actual code fix for the crash it's testing against. Merging this as-is could introduce a failing test. I've suggested a more descriptive name for the test to improve clarity, in line with our style guide's emphasis on readability. The test's widget tree can be simplified by removing a redundant Center widget, following the 'write what you need and no more' principle. I've referenced the Flutter Style Guide where applicable in my comments.

Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

LGTM

@dkwingsmt dkwingsmt self-requested a review August 27, 2025 18:23
Copy link
Contributor

@MitchellGoodwin MitchellGoodwin left a comment

Choose a reason for hiding this comment

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

LGTM as well!

Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

Some changes per the latest discussion

expect(secondaryOffset.dy - tileOffset.dy, bottomPositionSecondary);
});

testWidgets('CheckboxListTile renders at zero area', (WidgetTester tester) async {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
testWidgets('CheckboxListTile renders at zero area', (WidgetTester tester) async {
testWidgets('CheckboxListTile does not crash at zero area', (WidgetTester tester) async {

Comment on lines 1861 to 1862
final Finder xText = find.text('X');
expect(tester.getSize(xText).isEmpty, isTrue);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
final Finder xText = find.text('X');
expect(tester.getSize(xText).isEmpty, isTrue);
expect(tester.getSize(find.byType(CheckboxListTile)), Size.zero);

@ahmedsameha1 ahmedsameha1 force-pushed the handle#6537-CheckboxListTile branch from 8f7dac0 to 5635d14 Compare September 4, 2025 09:41
Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

Checklist:

  • The test is in the correct file
  • The test name goes “does not crash at zero area”
  • The target widget is wrapped by Center (or is fullscreen)
  • The target widget does not have an overlay, or the overlay is tested
  • The target widget is expected to have a size of exactly Size.zero

@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 18, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 18, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Sep 18, 2025

autosubmit label was removed for flutter/flutter/173151, because - The status or check suite Mac_x64 tool_tests_commands has failed. Please fix the issues identified (or deflake) before re-applying this label.

@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 23, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 23, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Sep 23, 2025

autosubmit label was removed for flutter/flutter/173151, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@MitchellGoodwin MitchellGoodwin force-pushed the handle#6537-CheckboxListTile branch from 82e7400 to 7d83d25 Compare October 1, 2025 17:27
@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 1, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Oct 1, 2025

autosubmit label was removed for flutter/flutter/173151, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 1, 2025
@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 8, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 8, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Oct 8, 2025

autosubmit label was removed for flutter/flutter/173151, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@dkwingsmt dkwingsmt force-pushed the handle#6537-CheckboxListTile branch from b0400a6 to 0481794 Compare October 8, 2025 23:21
@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 8, 2025
@dkwingsmt dkwingsmt force-pushed the handle#6537-CheckboxListTile branch from 0481794 to 5859547 Compare October 16, 2025 21:49
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 16, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Oct 16, 2025

autosubmit label was removed for flutter/flutter/173151, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 2, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 2, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Nov 2, 2025

autosubmit label was removed for flutter/flutter/173151, because - The status or check suite Linux web_long_running_tests_3_5 has failed. Please fix the issues identified (or deflake) before re-applying this label.

@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 2, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Nov 2, 2025
Merged via the queue into flutter:master with commit 604bc86 Nov 2, 2025
79 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 2, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 4, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 4, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 4, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 4, 2025
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
…lutter#173151)

This is my attempt to handle
flutter#6537 for the CheckboxListTile
UI control.

Co-authored-by: Tong Mu <[email protected]>
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.

3 participants