Sitelet https://github.com/flutter/flutter/pull/187058
Skip to content

Fix Cupertino sheet covered top gap transition - #187058

Closed
huahua8893 wants to merge 1 commit into
flutter:masterfrom
huahua8893:fix-cupertino-sheet
Closed

huahua8893 wants to merge 1 commit into
flutter:masterfrom
huahua8893:fix-cupertino-sheet

Conversation

@huahua8893

@huahua8893 huahua8893 commented May 25, 2026 •

Copy link
Copy Markdown

Fixes #187057

This fixes a visual issue in stacked CupertinoSheetRoutes where the top gap of a covered sheet can reveal a lower route.

When a sheet is covered by another sheet, the covered sheet is translated and scaled by its secondary route animation. However, its top gap was outside that secondary transition. In a stack such as:

Root route -> Sheet 2 -> Sheet 3

the top gap could remain visually separate from the covered sheet's transition, allowing the root route background to show through at the top of the sheet stack.

This change applies the covered sheet's secondary transition outside its top gap padding, so the covered sheet and its top gap move and scale together.

A regression test was added to verify that a covered sheet applies its secondary transition outside the top gap.

Tests

flutter test packages/flutter/test/cupertino/sheet_test.dart
dart analyze packages/flutter/lib/src/cupertino/sheet.dart packages/flutter/test/cupertino/sheet_test.dart

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools.
  • 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].
  • I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
  • All existing and new tests are passing.

Problem

When multiple CupertinoSheetRoutes are stacked, the covered sheet's top gap can reveal a lower route.

For example:

Root route -> Sheet 2 -> Sheet 3

If the root route has a distinct background color, that color can become visible above the covered sheet. This makes the sheet stack look incorrect because the root route should remain hidden behind the sheet stack.

Videos

Before this PR:

2026-05-25.22.32.33.mov

After this PR:

2026-05-25.22.28.59.mov

@github-actions github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. p: cupertino_ui cupertino_ui package in flutter/packages labels May 25, 2026

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

Copy link
Copy Markdown
Contributor

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 updates the Cupertino sheet transition in sheet.dart to include a black background behind the sheet during secondary transitions by utilizing a Stack and a FadeTransition. A new widget test has been added to verify that this opaque background is correctly rendered when multiple sheets are pushed. I have no feedback to provide.

@dkwingsmt
dkwingsmt self-requested a review May 27, 2026 18:19

@victorsanni victorsanni left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the PR. Can you add a video of native iOS in the same circumstances to the video description? and also what the previous (wrong) behavior this PR is fixing too? While the latter is in the issue it may be helpful to do a side-by-side comparison here also.

Comment thread packages/flutter/lib/src/cupertino/sheet.dart Outdated
@huahua8893
huahua8893 force-pushed the fix-cupertino-sheet branch from a1ed79d to d02a69b Compare June 3, 2026 03:44
@huahua8893 huahua8893 changed the title Fix Cupertino sheet covered top gap background Fix Cupertino sheet covered top gap transition Jun 3, 2026
@Piinks Piinks added the Decoupling: Not ready to port yet Instructions will be provided when this is ready to move to flutter/packages. label Jun 24, 2026
@Piinks

Piinks commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

I've marked this PR as not ready to port to flutter/packages yet.
We'll provide instructions to move this change over to material_ui/cupertino_ui once ready to receive PRs. Thank you!

@huahua8893

Copy link
Copy Markdown
Author

I've marked this PR as not ready to port to flutter/packages yet. We'll provide instructions to move this change over to material_ui/cupertino_ui once ready to receive PRs. Thank you!

Thanks for the update! Sounds good. I'll wait for the instructions and move this change over to material_ui/cupertino_ui once they are ready to receive PRs.

@Piinks
Piinks requested a review from victorsanni July 15, 2026 21:11
Comment thread packages/flutter/test/cupertino/sheet_test.dart Outdated
@huahua8893
huahua8893 force-pushed the fix-cupertino-sheet branch from d02a69b to f8d3c0a Compare August 6, 2026 08:44
@victorsanni
victorsanni self-requested a review August 6, 2026 16:16
victorsanni
victorsanni previously approved these changes Aug 6, 2026

@dkwingsmt dkwingsmt left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the investigation! The fix looks good to me, however I'm not a fan of this unit test. Checking pixels from a screenshot is unsupported on Web, less performative, and might even be flaky, and should only be used if a regular coordinate test isn't possible.

What do you think of a test like this?

  testWidgets('covered sheet does not reveal the root route through its top gap', (
    WidgetTester tester,
  ) async {
    final GlobalKey scaffoldKey = GlobalKey();

    await tester.pumpWidget(
      CupertinoApp(
        home: CupertinoPageScaffold(
          key: scaffoldKey,
          child: Column(
            children: <Widget>[
              const Text('Page 1'),
              CupertinoButton(
                onPressed: () {
                  Navigator.push<void>(
                    scaffoldKey.currentContext!,
                    CupertinoSheetRoute<void>(
                      builder: (BuildContext context) {
                        return CupertinoPageScaffold(
                          child: Column(
                            children: <Widget>[
                              const Text('Page 2'),
                              CupertinoButton(
                                onPressed: () {
                                  Navigator.push<void>(
                                    context,
                                    CupertinoSheetRoute<void>(
                                      builder: (BuildContext context) {
                                        return const CupertinoPageScaffold(child: Text('Page 3'));
                                      },
                                    ),
                                  );
                                },
                                child: const Text('Push Page 3'),
                              ),
                            ],
                          ),
                        );
                      },
                    ),
                  );
                },
                child: const Text('Push Page 2'),
              ),
            ],
          ),
        ),
      ),
    );

    await tester.tap(find.text('Push Page 2'));
    await tester.pumpAndSettle();

    await tester.tap(find.text('Push Page 3'));
    await tester.pumpAndSettle();

    final double page1Top = tester
        .getTopLeft(
          find.ancestor(of: find.text('Page 1'), matching: find.byType(CupertinoPageScaffold)),
        )
        .dy;
    final double page2Top = tester
        .getTopLeft(
          find.ancestor(of: find.text('Page 2'), matching: find.byType(CupertinoPageScaffold)),
        )
        .dy;
    final double page3Top = tester
        .getTopLeft(
          find.ancestor(of: find.text('Page 3'), matching: find.byType(CupertinoPageScaffold)),
        )
        .dy;

    // Sheet 2 moves up above Page 1 so that the root route (Page 1) is completely hidden behind Sheet 2.
    expect(page2Top, lessThanOrEqualTo(page1Top));
    // Sheet 3 is stacked on top of Sheet 2, with Sheet 2 peeking out above Sheet 3.
    expect(page2Top, lessThanOrEqualTo(page3Top));
  });

@github-actions

Copy link
Copy Markdown

This pull request contains changes to Material or Cupertino, which are currently frozen in this repository.

Changes should be made in material_ui and/or cupertino_ui in the flutter/packages repository.

Please refer to #188444 for instructions.

@huahua8893

huahua8893 commented Aug 19, 2026 •

Copy link
Copy Markdown
Author

@dkwingsmt Sorry for the delayed. This approach is more robust and broadly applicable than the previous test. I have updated the regression test to use the suggested coordinate-based assertions and verified that it fails with the previous implementation and passes with the fix, confirming that it accurately captures the issue. I also learned a valuable testing approach from your suggestion. Thank you very much for the thoughtful feedback!

PS:I have noted the recent migration to the new flutter/packages repository, and I will port this PR accordingly by following the provided instructions.

@dkwingsmt

dkwingsmt commented Aug 19, 2026 •

Copy link
Copy Markdown
Contributor

@huahua8893 Yes, can you port the changes to the packages, request the same reviewers, and close this PR? Thank you!

@huahua8893

Copy link
Copy Markdown
Author

@dkwingsmt @victorsanni Thank you for the reviews and guidance. I have ported this change to flutter/packages#12530 and requested the same reviewers there. I’m closing this PR in favor of the migrated PR.

@huahua8893 huahua8893 closed this Aug 21, 2026
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Aug 25, 2026
…#12530)

Ports flutter/flutter#187058 to `cupertino_ui` following flutter/flutter#188444.

Fixes flutter/flutter#187057.

When multiple `CupertinoSheetRoute`s are stacked, the covered sheet's top gap can reveal the root route because the top-gap padding sits outside the secondary route transition.

This change applies the covered sheet's secondary transition outside the top-gap padding, so the sheet and its gap move together. It also adds a coordinate-based regression test that verifies the root route remains fully covered.

## Tests

- `flutter test test/sheet_test.dart --no-pub`
- `dart run script/tool/bin/flutter_plugin_tools.dart analyze --packages cupertino_ui`
- `dart run script/tool/bin/flutter_plugin_tools.dart validate --packages cupertino_ui --base-sha=252bb33ad3666c7d28621c87edccafff86e210fd --check-for-missing-changes`
- `dart run script/tool/bin/flutter_plugin_tools.dart publish-check --packages cupertino_ui`

## Pre-Review Checklist
danielleon-cmd pushed a commit to victogomez-cs/packages-fork that referenced this pull request Aug 27, 2026
…flutter#12530)

Ports flutter/flutter#187058 to `cupertino_ui` following flutter/flutter#188444.

Fixes flutter/flutter#187057.

When multiple `CupertinoSheetRoute`s are stacked, the covered sheet's top gap can reveal the root route because the top-gap padding sits outside the secondary route transition.

This change applies the covered sheet's secondary transition outside the top-gap padding, so the sheet and its gap move together. It also adds a coordinate-based regression test that verifies the root route remains fully covered.

## Tests

- `flutter test test/sheet_test.dart --no-pub`
- `dart run script/tool/bin/flutter_plugin_tools.dart analyze --packages cupertino_ui`
- `dart run script/tool/bin/flutter_plugin_tools.dart validate --packages cupertino_ui --base-sha=252bb33ad3666c7d28621c87edccafff86e210fd --check-for-missing-changes`
- `dart run script/tool/bin/flutter_plugin_tools.dart publish-check --packages cupertino_ui`

## Pre-Review Checklist
jagadeesh8682 pushed a commit to jagadeesh8682/packages that referenced this pull request Sep 2, 2026
…flutter#12530)

Ports flutter/flutter#187058 to `cupertino_ui` following flutter/flutter#188444.

Fixes flutter/flutter#187057.

When multiple `CupertinoSheetRoute`s are stacked, the covered sheet's top gap can reveal the root route because the top-gap padding sits outside the secondary route transition.

This change applies the covered sheet's secondary transition outside the top-gap padding, so the sheet and its gap move together. It also adds a coordinate-based regression test that verifies the root route remains fully covered.

## Tests

- `flutter test test/sheet_test.dart --no-pub`
- `dart run script/tool/bin/flutter_plugin_tools.dart analyze --packages cupertino_ui`
- `dart run script/tool/bin/flutter_plugin_tools.dart validate --packages cupertino_ui --base-sha=252bb33ad3666c7d28621c87edccafff86e210fd --check-for-missing-changes`
- `dart run script/tool/bin/flutter_plugin_tools.dart publish-check --packages cupertino_ui`

## Pre-Review Checklist
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Decoupling: Not ready to port yet Instructions will be provided when this is ready to move to flutter/packages. f: cupertino framework flutter/packages/flutter repository. See also f: labels. p: cupertino_ui cupertino_ui package in flutter/packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CupertinoSheetRoute top gap can reveal lower routes when multiple sheets are stacked

4 participants