Feat: Add top gap for cupertino sheet #171348

rkishan516 · 2025-06-29T03:08:23Z

Feat: Add top gap for cupertino sheet
fixes: #169465

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].
I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
All existing and new tests are passing.

flutter-dashboard · 2025-06-29T03:08:27Z

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

MitchellGoodwin · 2025-07-01T22:18:43Z

I'm not sure the top gap should be set as a pixel value. Instead it might be better as a ratio of the screen height, from 0 to 1.0. So 0.5 would be half screen. That would be safer with looking correct across different screen sizes.

MitchellGoodwin

Thank you for putting this together. Overall I like the approach, but this causes a problem with stacking multiple sheets, that I mention below. I'm not sure how best to approach that.

In the future, we'll likely want to add detents to the sheet: #169832. I was wondering if we should not add a topGap value and leave that to be handled by the more complicated detents, but I think it would both be good to have a simple value to set, and this could function as an initial sheet size to open on when using detents. So I think we should continue with this for now.

packages/flutter/lib/src/cupertino/sheet.dart

MitchellGoodwin · 2025-07-02T17:27:50Z

packages/flutter/lib/src/cupertino/sheet.dart

  final bool enableDrag;

+  @override
+  final double? topGap;


I think we could make this a little friendlier. We can add a private _topGap value. The constructor sets _topGap to whatever user defined property, or leaves it null. Then this override only sets a getter topGap that returns _topGap ?? _kTopGapRatio. Then we can both just use topGap everywhere and if another bit of code looks at topGap than it will be a reliable source of truth.

Also, I think we don't want to expose a setter for now. In the future we will probably want to enable programmatically changing the top gap while the sheet is open and having it animate the difference, and leaving no open setter for now makes that easier.

Pushed the change.

MitchellGoodwin

Looks like the test is still failing.

But it looks like in SwiftUI the stacked view only happens with the default sheet height, or .large for them. All others have the previous page shown normally.

I think that simplifies what we need to do. If a user provides a topGap, then we don't want the previous route to transition at all, even if it matches our default. So I think we can do this:

Create a private double? _topGap. The constructor sets _topGap to the user provided topGap, if provided. This lets us track if it's left as null.
Add a topGap getter for providing programmatically which topGap to use.

double get topGap => _topGap ?? _kTopGapRatio

Add a canTransitionFrom override and have both it and canTransitionTo return false if _topGap != null

I think if we do all that, we can revert delegatedTransition to what it was before and not worry about it. It won't be used at all if exit transitions are blocked from the code above.

rkishan516 · 2025-07-16T02:47:33Z

@MitchellGoodwin Reverted the delegatedTransition Change and updated canTransition methods with topGap, but i think test are still failing, I tested it on device, looks fine but might have missed any edge case.

MitchellGoodwin

Thank you for the update, I left some comments.

MitchellGoodwin · 2025-07-23T17:35:49Z

packages/flutter/lib/src/cupertino/sheet.dart

+        Animation<double> secondaryAnimation,
+        bool allowSnapshotting,
+        Widget? child,
+      ) => CupertinoSheetTransition.delegateTransition(


I think this can be simplified to

_topGap != null ? null : CupertinoSheetTransition.delegateTransition;

MitchellGoodwin · 2025-07-23T17:40:00Z

packages/flutter/lib/src/cupertino/sheet.dart

+    if (this is CupertinoSheetRoute<dynamic>) {
+      return (this as CupertinoSheetRoute<dynamic>)._topGap == null;
+    }


Can we get this logic without the type casting?

MitchellGoodwin · 2025-07-23T17:40:53Z

packages/flutter/lib/src/cupertino/sheet.dart

-    Widget? child,
-  ) {
+    Widget? child, {
+    double? topGap,


Since we won't use this transition if there is a topGap, then we can remove this and not handle it.

erkinovalim · 2025-08-18T23:05:59Z

Are there any updates on this? Could we get this merged at this point?

rkishan516 · 2025-08-19T01:21:30Z

@erkinovalim We still need review.

MitchellGoodwin

Sorry it took a while to get back to this. Approach looks good, I have some nitpick comments on the canTransitionTo and canTransitionFrom methods. Main thing left is adding a test.

packages/flutter/lib/src/cupertino/sheet.dart

erkinovalim · 2025-09-21T15:04:28Z

@MitchellGoodwin could you review this please?

MitchellGoodwin

Thank you for the updates. It looks like the top gap value isn't actually being used for the sheet itself. The topGap needs to be applied to the _stretchDragAnimation in order to be used. However, I'm not sure how stretching should work with a custom top gap. Either the value for the max stretch needs to adjust if a custom top gap is given, or stretching needs to be disabled with a custom top gap.

MitchellGoodwin · 2025-10-01T17:01:43Z

packages/flutter/lib/src/cupertino/sheet.dart

  bool get enableDrag;

+  /// The gap between the top of the screen and the top of the sheet as a ratio
+  /// of the screen height (0.0 to 1.0). Defaults to [_kTopGapRatio].


Private variables won't be able to be seen by documentation.

Suggested change

/// of the screen height (0.0 to 1.0). Defaults to [_kTopGapRatio].

/// of the screen height (0.0 to 1.0). Defaults to a value of 0.08.

Sure, will update.

MitchellGoodwin · 2025-10-01T17:23:51Z

packages/flutter/lib/src/cupertino/sheet.dart

+  double get topGap => _topGap ?? _kTopGapRatio;
+
+  @override
+  bool get hasCustomTopGap => _topGap != null;


This maybe should be a private method. I'm not sure if it's useful outside of these classes.

This is used inside mixin. Here, we are just providing value. So, updated to private.

erkinovalim · 2025-10-17T15:20:57Z

@MitchellGoodwin could you review this please?

MitchellGoodwin

Apologies for the late review. Tested this locally and it lgtm. Left two comments, but other than that, I think this is good to go.

MitchellGoodwin · 2025-11-05T17:45:08Z

packages/flutter/lib/src/cupertino/sheet.dart

+    if (this is CupertinoSheetRoute<dynamic> && _hasCustomTopGap) {
+      return false;
+    }
+    return nextRoute is _CupertinoSheetRouteTransitionMixin && !_hasCustomTopGap;


This check for !_hasCustomTopGap is redundant, because it already happens above.

Sure, will do.

packages/flutter/lib/src/cupertino/sheet.dart

MitchellGoodwin · 2025-11-06T18:18:02Z

packages/flutter/lib/src/cupertino/sheet.dart

+    this.enableDrag = true,
+    double? topGap,
+  }) : assert(
+         topGap == null || (topGap >= 0.0 && topGap <= 1.0),


I wonder if we should limit the topGap so that it is not able to be 1.0, or the gap is the whole screen height. That would mean that none of the sheet is visible. I don't know why somebody would do that, but if they did it would mean that the top most route is active, but not on the screen at all, which would be hacky and also seems like it would mess with accessibility. Maybe it should be a max of 0.9, or 0.95?

If they set the topgap to 0.0 though, so that the sheet is fullscreen, I think that's perfectly ok.

Yaa, we can do 0.9 because even 10% will be too small.

What is the behavior like on swiftui?

MitchellGoodwin · 2025-11-12T17:51:06Z

packages/flutter/lib/src/cupertino/sheet.dart

  bool enableDrag = true,
+  double? topGap,
 }) {
+  assert(topGap == null || (topGap >= 0.0 && topGap <= 1.0), 'topGap must be between 0.0 and 1.0');


Need the check for < 0.9 here as well

Sorry, I should have updated this also. Pushing change now.

MitchellGoodwin

LGTM, pending some documentation nits. Thank you so much for your patience on this one.

MitchellGoodwin · 2025-12-10T18:20:38Z

packages/flutter/lib/src/cupertino/sheet.dart

+/// It should be a value between 0.0 and 1.0, where 0.0 means no gap and 1.0
+/// means the sheet starts at the bottom. If not provided, defaults to 0.08


Suggested change

/// It should be a value between 0.0 and 1.0, where 0.0 means no gap and 1.0

/// means the sheet starts at the bottom. If not provided, defaults to 0.08

/// It should be a value between 0.0 and 0.9, where 0.0 means no gap and 0.9

/// means the sheet takes up only the bottom 10% of the screen. If not provided, defaults to 0.08

MitchellGoodwin · 2025-12-10T18:22:00Z

packages/flutter/lib/src/cupertino/sheet.dart

+  /// This value should be between 0.0 and 1.0, where 0.0 means no gap (sheet
+  /// extends to the top of the screen) and 1.0 means the sheet starts at the
+  /// bottom of the screen. A value of 0.08 represents 8% of the screen height.


Suggested change

/// This value should be between 0.0 and 1.0, where 0.0 means no gap (sheet

/// extends to the top of the screen) and 1.0 means the sheet starts at the

/// bottom of the screen. A value of 0.08 represents 8% of the screen height.

/// This value should be between 0.0 and 0.9, where 0.0 means no gap (sheet

/// extends to the top of the screen) and 0.9 means the sheet covers only the

/// bottom 10% of the screen. A value of 0.08 represents 8% of the screen height.

MitchellGoodwin · 2025-12-10T18:22:28Z

packages/flutter/lib/src/cupertino/sheet.dart

      duration: const Duration(microseconds: 1),
      vsync: this,
    );
+    // Maintain the same stretch distance (0.008 of screen height) regardless of custom topGap


Suggested change

// Maintain the same stretch distance (0.008 of screen height) regardless of custom topGap

// Maintain the same stretch distance (0.008 of screen height) regardless of custom topGap.

MitchellGoodwin · 2025-12-10T18:22:51Z

packages/flutter/lib/src/cupertino/sheet.dart

      // Divide by stretchable range (when dragging upward at max extent).
-      upController.value -=
-          delta / (navigator.context.size!.height * (_kTopGapRatio - _kStretchedTopGapRatio));
+      // Maintain the same stretch distance regardless of custom topGap


Suggested change

// Maintain the same stretch distance regardless of custom topGap

// Maintain the same stretch distance regardless of custom topGap.

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: cupertino flutter/packages/flutter/cupertino repository labels Jun 29, 2025

rkishan516 force-pushed the cupertino-sheet-top-gap branch from 3154d0a to cd81ee1 Compare July 2, 2025 04:53

MitchellGoodwin reviewed Jul 2, 2025

View reviewed changes

rkishan516 force-pushed the cupertino-sheet-top-gap branch 2 times, most recently from 7dac230 to 92dec8d Compare July 12, 2025 13:31

MitchellGoodwin reviewed Jul 15, 2025

View reviewed changes

rkishan516 force-pushed the cupertino-sheet-top-gap branch from b92aa43 to 91fe520 Compare July 16, 2025 02:45

MitchellGoodwin reviewed Jul 23, 2025

View reviewed changes

rkishan516 force-pushed the cupertino-sheet-top-gap branch from 91fe520 to 6797926 Compare July 24, 2025 01:58

rkishan516 force-pushed the cupertino-sheet-top-gap branch 2 times, most recently from aa3f128 to a4f1b27 Compare August 19, 2025 02:24

dkwingsmt requested a review from MitchellGoodwin August 20, 2025 18:38

MitchellGoodwin reviewed Sep 3, 2025

View reviewed changes

packages/flutter/lib/src/cupertino/sheet.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/cupertino/sheet.dart Show resolved Hide resolved

rkishan516 force-pushed the cupertino-sheet-top-gap branch from a4f1b27 to 40a4bc9 Compare September 4, 2025 08:46

dkwingsmt requested a review from MitchellGoodwin September 17, 2025 18:28

MitchellGoodwin reviewed Oct 1, 2025

View reviewed changes

rkishan516 force-pushed the cupertino-sheet-top-gap branch 2 times, most recently from 2c027e4 to 5d263d7 Compare October 2, 2025 01:58

rkishan516 requested a review from MitchellGoodwin October 7, 2025 02:42

MitchellGoodwin reviewed Nov 5, 2025

View reviewed changes

rkishan516 force-pushed the cupertino-sheet-top-gap branch from 5d263d7 to c8f49ae Compare November 6, 2025 01:39

MitchellGoodwin reviewed Nov 6, 2025

View reviewed changes

rkishan516 force-pushed the cupertino-sheet-top-gap branch from c8f49ae to a720971 Compare November 7, 2025 01:07

MitchellGoodwin reviewed Nov 12, 2025

View reviewed changes

rkishan516 force-pushed the cupertino-sheet-top-gap branch from a720971 to 9f0fed7 Compare November 13, 2025 01:17

rkishan516 force-pushed the cupertino-sheet-top-gap branch from 9f0fed7 to 272df29 Compare November 26, 2025 03:27

MitchellGoodwin approved these changes Dec 10, 2025

View reviewed changes

rkishan516 added 5 commits December 11, 2025 16:26

feat: add top gap ratio

dcaf8f8

refactor: remove redundent condition of _hasCustomTopGap

9566fab

docs: enhanced topGap perameter doc

d52f785

fix: update top gap assertion for showCupertinoSheet

0cb15df

docs: update docs for max top gap value based on feedback

e9bf03f

rkishan516 force-pushed the cupertino-sheet-top-gap branch from 272df29 to e9bf03f Compare December 11, 2025 11:04

rkishan516 added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 16, 2025

auto-submit bot added this pull request to the merge queue Dec 16, 2025

Merged via the queue into flutter:master with commit 698615f Dec 16, 2025
71 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Dec 16, 2025

engine-flutter-autoroll mentioned this pull request Dec 17, 2025

Roll Flutter from 6e1aa823523d to 57c3f8b66525 (85 revisions) flutter/packages#10638

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 17, 2025

698615f94 Feat: Add top gap for cupertino sheet (flutter/flutter#171348)

5dae84f

engine-flutter-autoroll mentioned this pull request Dec 17, 2025

Roll Flutter from 6e1aa823523d to 4eb454d5804e (86 revisions) flutter/packages#10639

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 17, 2025

698615f94 Feat: Add top gap for cupertino sheet (flutter/flutter#171348)

92b12cc

engine-flutter-autoroll mentioned this pull request Dec 17, 2025

Roll Flutter from 6e1aa823523d to cdd30e7ec6f1 (87 revisions) flutter/packages#10640

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 17, 2025

698615f94 Feat: Add top gap for cupertino sheet (flutter/flutter#171348)

58ab21f

engine-flutter-autoroll mentioned this pull request Dec 18, 2025

Roll Flutter from 6e1aa823523d to 4717a6489841 (103 revisions) flutter/packages#10642

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 18, 2025

698615f94 Feat: Add top gap for cupertino sheet (flutter/flutter#171348)

8121c81

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 18, 2025

698615f94 Feat: Add top gap for cupertino sheet (flutter/flutter#171348)

4226fdf

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 18, 2025

698615f94 Feat: Add top gap for cupertino sheet (flutter/flutter#171348)

84c262f

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 18, 2025

698615f94 Feat: Add top gap for cupertino sheet (flutter/flutter#171348)

39a861b

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 18, 2025

698615f94 Feat: Add top gap for cupertino sheet (flutter/flutter#171348)

fba3650

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 18, 2025

698615f94 Feat: Add top gap for cupertino sheet (flutter/flutter#171348)

16b2148

	/// of the screen height (0.0 to 1.0). Defaults to [_kTopGapRatio].
	/// of the screen height (0.0 to 1.0). Defaults to a value of 0.08.

		/// It should be a value between 0.0 and 1.0, where 0.0 means no gap and 1.0
		/// means the sheet starts at the bottom. If not provided, defaults to 0.08

	// Maintain the same stretch distance (0.008 of screen height) regardless of custom topGap
	// Maintain the same stretch distance (0.008 of screen height) regardless of custom topGap.

	// Maintain the same stretch distance regardless of custom topGap
	// Maintain the same stretch distance regardless of custom topGap.

Feat: Add top gap for cupertino sheet #171348

Feat: Add top gap for cupertino sheet #171348

Conversation

rkishan516 commented Jun 29, 2025

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented Jun 29, 2025

Uh oh!

MitchellGoodwin commented Jul 1, 2025

Uh oh!

MitchellGoodwin left a comment

Choose a reason for hiding this comment

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!

MitchellGoodwin left a comment

Choose a reason for hiding this comment

Uh oh!

rkishan516 commented Jul 16, 2025

Uh oh!

MitchellGoodwin left a comment

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!

erkinovalim commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rkishan516 commented Aug 19, 2025

Uh oh!

MitchellGoodwin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

erkinovalim commented Sep 21, 2025

Uh oh!

MitchellGoodwin left a comment

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!

rkishan516 Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erkinovalim commented Oct 17, 2025

Uh oh!

MitchellGoodwin left a comment

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!

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!

MitchellGoodwin left a comment

erkinovalim commented Aug 18, 2025 •

edited

Loading

rkishan516 Oct 2, 2025 •

edited

Loading