From ccae8cc79480c854992cd093cf5b43bd4a9b50c9 Mon Sep 17 00:00:00 2001 From: Mitchell Goodwin <58190796+MitchellGoodwin@users.noreply.github.com> Date: Tue, 4 Feb 2025 10:33:57 -0800 Subject: [PATCH] Adjust padding for Cupertino sheet content (#162481) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #162215 and partially supports #162181. The Cupertino sheet had two issues layout: It still had MediaQuery padding at the top of the screen as if there was safe area content to avoid and some of the sheet was permanently offscreen. This caused a FloatingActionButton to be clipped, but if you put a scrolling widget in the sheet with text at the very bottom, it was impossible to scroll the text into view. This PR removes the top padding, and changes the bottom padding to equal the amount offscreen. Before: Screenshot 2025-01-30 at 11 21 36 AM After: Screenshot 2025-01-30 at 11 19 54 AM ## 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. - [ ] 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]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [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 --- packages/flutter/lib/src/cupertino/sheet.dart | 29 ++++-- .../flutter/test/cupertino/sheet_test.dart | 96 +++++++++++++++++++ 2 files changed, 117 insertions(+), 8 deletions(-) diff --git a/packages/flutter/lib/src/cupertino/sheet.dart b/packages/flutter/lib/src/cupertino/sheet.dart index ef2453b13b0..f2eca5e196e 100644 --- a/packages/flutter/lib/src/cupertino/sheet.dart +++ b/packages/flutter/lib/src/cupertino/sheet.dart @@ -11,13 +11,18 @@ import 'interface_level.dart'; import 'route.dart'; import 'theme.dart'; +// The distance from the top of the open sheet to the top of the screen, as a ratio +// of the total height of the screen. Found from eyeballing a simulator running +// iOS 18.0. +const double _kTopGapRatio = 0.08; + // Tween for animating a Cupertino sheet onto the screen. // // Begins fully offscreen below the screen and ends onscreen with a small gap at // the top of the screen. Values found from eyeballing a simulator running iOS 18.0. final Animatable _kBottomUpTween = Tween( begin: const Offset(0.0, 1.0), - end: const Offset(0.0, 0.08), + end: const Offset(0.0, _kTopGapRatio), ); // Offset change for when a new sheet covers another sheet. '0.0' represents the @@ -456,9 +461,19 @@ class CupertinoSheetRoute extends PageRoute with _CupertinoSheetRouteTrans @override Widget buildContent(BuildContext context) { - return CupertinoUserInterfaceLevel( - data: CupertinoUserInterfaceLevelData.elevated, - child: _CupertinoSheetScope(child: builder(context)), + final double bottomPadding = MediaQuery.sizeOf(context).height * _kTopGapRatio; + + return MediaQuery.removePadding( + context: context, + removeTop: true, + removeBottom: true, + child: Padding( + padding: EdgeInsets.only(bottom: bottomPadding), + child: CupertinoUserInterfaceLevel( + data: CupertinoUserInterfaceLevelData.elevated, + child: _CupertinoSheetScope(child: builder(context)), + ), + ), ); } @@ -638,11 +653,9 @@ class _CupertinoDownGestureDetectorState extends State<_CupertinoDownGestureD void _handleDragUpdate(DragUpdateDetails details) { assert(mounted); assert(_downGestureController != null); - final double topGapRatio = (_kBottomUpTween as Tween).end?.dy ?? 0.08; _downGestureController!.dragUpdate( - // Devide by size of the sheet. The gap between the top of the sheet and - // top of the screen is 0.08. - details.primaryDelta! / (context.size!.height - (context.size!.height * topGapRatio)), + // Divide by size of the sheet. + details.primaryDelta! / (context.size!.height - (context.size!.height * _kTopGapRatio)), ); } diff --git a/packages/flutter/test/cupertino/sheet_test.dart b/packages/flutter/test/cupertino/sheet_test.dart index 217b41c14a5..1b6b8e4bd4d 100644 --- a/packages/flutter/test/cupertino/sheet_test.dart +++ b/packages/flutter/test/cupertino/sheet_test.dart @@ -7,6 +7,9 @@ import 'package:flutter_test/flutter_test.dart'; import '../widgets/navigator_utils.dart'; +// Matches _kTopGapRatio in cupertino/sheet.dart. +const double _kTopGapRatio = 0.08; + void main() { testWidgets('Sheet route does not cover the whole screen', (WidgetTester tester) async { final GlobalKey scaffoldKey = GlobalKey(); @@ -649,6 +652,99 @@ void main() { expect(find.text('Page: /next'), findsOneWidget); }); + testWidgets('content does not go below the bottom of the screen', (WidgetTester tester) async { + final GlobalKey scaffoldKey = GlobalKey(); + + await tester.pumpWidget( + CupertinoApp( + home: CupertinoPageScaffold( + key: scaffoldKey, + child: Center( + child: Column( + children: [ + const Text('Page 1'), + CupertinoButton( + onPressed: () { + Navigator.push( + scaffoldKey.currentContext!, + CupertinoSheetRoute( + builder: (BuildContext context) { + return CupertinoPageScaffold(child: Container()); + }, + ), + ); + }, + child: const Text('Push Page 2'), + ), + ], + ), + ), + ), + ), + ); + + await tester.tap(find.text('Push Page 2')); + await tester.pumpAndSettle(); + + expect(tester.getSize(find.byType(Container)).height, 600.0 - (600.0 * _kTopGapRatio)); + }); + + testWidgets('nested navbars remove MediaQuery top padding', (WidgetTester tester) async { + final GlobalKey scaffoldKey = GlobalKey(); + final GlobalKey appBarKey = GlobalKey(); + final GlobalKey sheetBarKey = GlobalKey(); + + await tester.pumpWidget( + CupertinoApp( + home: MediaQuery( + data: const MediaQueryData(padding: EdgeInsets.fromLTRB(0, 20, 0, 0)), + child: CupertinoPageScaffold( + key: scaffoldKey, + navigationBar: CupertinoNavigationBar( + key: appBarKey, + middle: const Text('Navbar'), + backgroundColor: const Color(0xFFF8F8F8), + ), + child: Center( + child: Column( + children: [ + const Text('Page 1'), + CupertinoButton( + onPressed: () { + Navigator.push( + scaffoldKey.currentContext!, + CupertinoSheetRoute( + builder: (BuildContext context) { + return CupertinoPageScaffold( + navigationBar: CupertinoNavigationBar( + key: sheetBarKey, + middle: const Text('Navbar'), + ), + child: Container(), + ); + }, + ), + ); + }, + child: const Text('Push Page 2'), + ), + ], + ), + ), + ), + ), + ), + ); + final double homeNavBardHeight = tester.getSize(find.byKey(appBarKey)).height; + + await tester.tap(find.text('Push Page 2')); + await tester.pumpAndSettle(); + + final double sheetNavBarHeight = tester.getSize(find.byKey(sheetBarKey)).height; + + expect(sheetNavBarHeight, lessThan(homeNavBardHeight)); + }); + group('drag dismiss gesture', () { Widget dragGestureApp(GlobalKey homeScaffoldKey, GlobalKey sheetScaffoldKey) { return CupertinoApp(