-
Notifications
You must be signed in to change notification settings - Fork 29.7k
CupertinoSheetRoute with scrolling and dragging #177337
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
MitchellGoodwin
wants to merge
18
commits into
flutter:master
Choose a base branch
from
MitchellGoodwin:dragging-sheet
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,049
−60
Open
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
8817960
Add scrollableBuilder
MitchellGoodwin b3baddf
Refactor
MitchellGoodwin 2dc2156
fix check for scrollcontroller
MitchellGoodwin b37ddb9
Partial refactor to DSS style
MitchellGoodwin a498867
Change extent to delta
MitchellGoodwin c00a8b4
refactor to no extent
MitchellGoodwin 7d64cdc
Remove changes to draggablescrollable
MitchellGoodwin f1b984f
Clean up
MitchellGoodwin 18274ce
Drag Area and sample code
MitchellGoodwin b1fa3e7
Documentation
MitchellGoodwin c547a6f
Pass height instead of context and fix typo
MitchellGoodwin 2bbd40a
Tests
MitchellGoodwin aa989d3
Sample update and tests
MitchellGoodwin 29bc426
Documentation
MitchellGoodwin b0d7ced
Lints
MitchellGoodwin 5facacb
Dispose scroll controller
MitchellGoodwin 39ad4d3
formatting
MitchellGoodwin a9823b7
Address review feedback
MitchellGoodwin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
122 changes: 122 additions & 0 deletions
122
examples/api/lib/cupertino/sheet/cupertino_sheet.3.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| // Copyright 2014 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'package:flutter/cupertino.dart'; | ||
|
|
||
| /// Flutter code sample for [CupertinoSheetRoute]. | ||
|
|
||
| void main() { | ||
| runApp(const CupertinoSheetApp()); | ||
| } | ||
|
|
||
| class CupertinoSheetApp extends StatelessWidget { | ||
| const CupertinoSheetApp({super.key}); | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return const CupertinoApp( | ||
| title: 'Scrollable Cupertino Sheet', | ||
| home: HomePage(), | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| class HomePage extends StatelessWidget { | ||
| const HomePage({super.key}); | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return CupertinoPageScaffold( | ||
| navigationBar: const CupertinoNavigationBar( | ||
| middle: Text('Scrollable Cupertino Sheet Example'), | ||
| automaticBackgroundVisibility: false, | ||
| ), | ||
| child: Center( | ||
| child: Column( | ||
| mainAxisAlignment: MainAxisAlignment.center, | ||
| children: <Widget>[ | ||
| CupertinoButton.filled( | ||
| onPressed: () { | ||
| Navigator.of(context).push( | ||
| CupertinoSheetRoute<void>.scrollable( | ||
| scrollableBuilder: | ||
| (BuildContext context, ScrollController controller) => | ||
| _ScrollableSheetBody(scrollController: controller), | ||
| ), | ||
| ); | ||
| }, | ||
| child: const Text('Open Sheet'), | ||
| ), | ||
| ], | ||
| ), | ||
| ), | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| class _ScrollableSheetBody extends StatelessWidget { | ||
| const _ScrollableSheetBody({required this.scrollController}); | ||
|
|
||
| final ScrollController scrollController; | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return CupertinoPageScaffold( | ||
| navigationBar: CupertinoSheetNavbar( | ||
| child: CupertinoNavigationBar( | ||
| backgroundColor: CupertinoColors.systemGrey3, | ||
| middle: const Text('Scrollable Sheet'), | ||
| automaticBackgroundVisibility: false, | ||
| leading: CupertinoButton( | ||
| padding: EdgeInsets.zero, | ||
| child: const Text('Close'), | ||
| onPressed: () { | ||
| CupertinoSheetRoute.popSheet(context); | ||
| }, | ||
| ), | ||
| ), | ||
| ), | ||
| child: CustomScrollView( | ||
| controller: scrollController, | ||
| primary: false, | ||
| slivers: <Widget>[ | ||
| SliverList( | ||
| delegate: SliverChildBuilderDelegate(( | ||
| BuildContext context, | ||
| int index, | ||
| ) { | ||
| return Container( | ||
| alignment: Alignment.center, | ||
| height: 100, | ||
| child: const Text('Scroll Me'), | ||
| ); | ||
| }, childCount: 20), | ||
| ), | ||
| ], | ||
| ), | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| class CupertinoSheetNavbar extends StatelessWidget | ||
| implements ObstructingPreferredSizeWidget { | ||
| const CupertinoSheetNavbar({super.key, required this.child}); | ||
|
|
||
| final CupertinoNavigationBar child; | ||
|
|
||
| @override | ||
| bool shouldFullyObstruct(BuildContext context) { | ||
| return child.shouldFullyObstruct(context); | ||
| } | ||
|
|
||
| @override | ||
| Size get preferredSize { | ||
| return child.preferredSize; | ||
| } | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return CupertinoSheetDragArea(child: child); | ||
| } | ||
| } | ||
62 changes: 62 additions & 0 deletions
62
examples/api/test/cupertino/sheet/cupertino_sheet.3_test.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| // Copyright 2014 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'package:flutter/rendering.dart'; | ||
| import 'package:flutter_api_samples/cupertino/sheet/cupertino_sheet.3.dart' | ||
| as example; | ||
| import 'package:flutter_test/flutter_test.dart'; | ||
|
|
||
| void main() { | ||
| testWidgets('Tap on button displays cupertino sheet', ( | ||
| WidgetTester tester, | ||
| ) async { | ||
| await tester.pumpWidget(const example.CupertinoSheetApp()); | ||
|
|
||
| final Finder dialogTitle = find.text('Scrollable Sheet'); | ||
| expect(dialogTitle, findsNothing); | ||
|
|
||
| await tester.tap(find.text('Open Sheet')); | ||
| await tester.pumpAndSettle(); | ||
| expect(dialogTitle, findsOneWidget); | ||
|
|
||
| await tester.tap(find.text('Close')); | ||
| await tester.pumpAndSettle(); | ||
| expect(dialogTitle, findsNothing); | ||
| }); | ||
|
|
||
| testWidgets('Drag on nav bar triggers drag only', ( | ||
| WidgetTester tester, | ||
| ) async { | ||
| await tester.pumpWidget(const example.CupertinoSheetApp()); | ||
|
|
||
| final Finder dialogTitle = find.text('Scrollable Sheet'); | ||
| expect(dialogTitle, findsNothing); | ||
|
|
||
| await tester.tap(find.text('Open Sheet')); | ||
| await tester.pumpAndSettle(); | ||
| expect(dialogTitle, findsOneWidget); | ||
|
|
||
| final RenderBox box = | ||
| tester.renderObject(find.text('Scrollable Sheet')) as RenderBox; | ||
| final Offset navbarOffset = box.localToGlobal(Offset.zero); | ||
| final double initialSheetHeight = navbarOffset.dy; | ||
|
|
||
| final TestGesture gesture = await tester.startGesture(navbarOffset); | ||
| await gesture.moveBy(const Offset(0, -50)); | ||
| await tester.pump(); | ||
|
|
||
| // Upwards drag triggers stretch, and not scroll. | ||
| final double currentSheetHeight = box.localToGlobal(Offset.zero).dy; | ||
| expect(currentSheetHeight, lessThan(initialSheetHeight)); | ||
|
|
||
| await gesture.moveBy(const Offset(0, 200)); | ||
| await tester.pump(); | ||
|
|
||
| final double finalSheetHeight = box.localToGlobal(Offset.zero).dy; | ||
| expect(finalSheetHeight, greaterThan(initialSheetHeight)); | ||
|
|
||
| await gesture.up(); | ||
| await tester.pumpAndSettle(); | ||
| }); | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be automatically built into CupertinoNavigationBar?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible, and it would be convenient as this would theoretically be a common use case. I'd just worry about this behavior being discoverable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does feel a bit kludgy from the developer experience side.
What native behavior are we trying to emulate by exposing this drag wrapper for non-scrolling parts? If I had a header that I wanted to have drag the sheet, why not put it in a pinned sliver in a CustomScrollView as part of the scrollable? Would that achieve the same effect without adding more API for the developer to have to wire up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to be able to trigger the drag-to-dismiss behavior when a drag originates outside of the scroll area, and only the drag-to-dismiss, not a scroll. For example, if you start a drag on the header of a native sheet, it will not scroll the content below it, no matter what direction you drag. This is also the only way to trigger the stretch upwards drag effect on a sheet. If you start the drag from within the scrolling area then the draggable/scrollable behavior would happen.
I believe in that case then a scroll would be triggered on a drag upwards originating in the pinned sliver, instead of the stretch animation.
From native: a drag starting within the navbar will not scroll and will stretch upwards. A drag starting below the navbar will scroll depending on the direction, and will not scroll upwards.

Basically outside of the scrolling area we want the sheet to work as if it's a non-scrolling sheet. This wrapper lets a developer wrap the original vertical drag gesture recognizer only around those areas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be easier for the developer if, in the case of a scrollable sheet, we provide a header parameter and we can manage the gesture resolution ourselves internally?
What is the native experience like for developers building the UI you described above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you try it?
We have control over how the scrolling offset is applied here with the custom scroll position class in the scrollable sheet, could we divert it in this case?