Skip to content

Conversation

@ahmedsameha1
Copy link
Contributor

This is my attempt to handle #6537 for the Scaffold widget.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Oct 29, 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 pull request adds a regression test to ensure Scaffold does not crash when rendered in a 0x0 environment. The test is a great addition, but as implemented, it doesn't fully exercise the code path that caused the original crash. I've left a comment with a suggested change to make the test more effective.

Also, as a minor process note, please remember to include the pre-launch checklist in the pull request description as per the repository's contributing guidelines.

Comment on lines 3680 to 3691
testWidgets('Scaffold does not crash at zero area', (WidgetTester tester) async {
await tester.pumpWidget(
const MaterialApp(
home: Center(child: SizedBox.shrink(child: Scaffold())),
),
);
expect(tester.getSize(find.byType(Scaffold)), Size.zero);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The current test uses a Scaffold with no body. This means the code path that was causing the crash (_ScaffoldLayout.performLayout's body layout logic) is not executed, as it's guarded by if (hasChild(_ScaffoldSlot.body)). To properly test the fix and prevent regressions, the Scaffold should have at least a body.

Including an appBar would make the test more robust by ensuring layout calculations with non-zero contentTop are also tested under 0x0 size conditions, which was part of the original issue.

Suggested change
testWidgets('Scaffold does not crash at zero area', (WidgetTester tester) async {
await tester.pumpWidget(
const MaterialApp(
home: Center(child: SizedBox.shrink(child: Scaffold())),
),
);
expect(tester.getSize(find.byType(Scaffold)), Size.zero);
});
testWidgets('Scaffold does not crash at zero area', (WidgetTester tester) async {
await tester.pumpWidget(
const MaterialApp(
home: Center(child: SizedBox.shrink(child: Scaffold(body: SizedBox(), appBar: AppBar()))),
),
);
expect(tester.getSize(find.byType(Scaffold)), Size.zero);
});

@victorsanni victorsanni requested a review from dkwingsmt October 30, 2025 22:16
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 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 5e348fd Nov 2, 2025
77 of 78 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 2, 2025
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
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