RefreshIndicator should not be shown when overscroll occurs due to inertia #72132
Conversation
flutter-dashboard
bot
added
f: material design
| // 1, Begin drag when the scrollable widget at the edge with zero scroll position. | ||
| // 2, Keep drag before overscroll occurs when the scrollable widget has | ||
| // a non-zero scroll position(do not release finger before overscroll). | ||
| return (notification is ScrollStartNotification || (notification is ScrollUpdateNotification && notification.dragDetails != null)) |
There was a problem hiding this comment.
Since you have refactor out this check, can you also implement the opt in behavior as well? We can have an widget parameter to control whether it can be pull out if it drag from non zero scroll offset?
There was a problem hiding this comment.
OK, BUT I still want to talk to @Hixie about this behavior. I want to know what he is thinking right now. I don't think the refresh indicator is easily pulled out by misoperation.It must continue to pull down a certain distance before it can be refreshed. What do you think,?
| @@ -501,7 +501,7 @@ void main() { | |||
| ); | |||
| }); | |||
|
|
|||
| testWidgets('Top RefreshIndicator showed when dragging from non-zero scroll position', (WidgetTester tester) async { | |||
| testWidgets('Top RefreshIndicator be shown when dragging from non-zero scroll position', (WidgetTester tester) async { | |||
There was a problem hiding this comment.
neither "showed" nor "be shown" are really correct grammar here, I recommend "shows". :-)
There was a problem hiding this comment.
Done.
@Hixie Hi, so what's your opinion on this #9690 (comment) now?
There was a problem hiding this comment.
copied from discord chat
looks like the material design spec doesn't say one way or the other... it seems weird to me to allow pull to refresh when you're just scrolling, since pull to refresh can cost money (it does network traffic) and generally be disruptive. I would not expect it to happen when you're just scrolling, it should be specifically when you're at the top already. I don't have an objection to a flag to enable it when you're just scrolling if that's a common request.
I think this is a valid argument, and I am also agree we should have flag to toggle it on and off.
| @@ -47,6 +47,17 @@ enum _RefreshIndicatorMode { | |||
| canceled, // Animating the indicator's fade-out after not arming. | |||
| } | |||
|
|
|||
| /// Used to configure the [RefreshIndicator] trigger mode. | |||
There was a problem hiding this comment.
Used to configure how [RefreshIndicator] can be triggered.
|
|
||
| /// The indicator can only trigger when the child's [Scrollable] descendant overscrolls | ||
| /// by drag and its initial scroll position is on the edge. | ||
| strict, |
There was a problem hiding this comment.
strict sounds too generic. we should be specific.
There was a problem hiding this comment.
Do you have any suggestions for this name？ ：）
There was a problem hiding this comment.
I am not an expert on this. here is my suggestion.
strict :onEdge, dragStartsAtScrollableEdge
loose: anywhere, dragStartsAtAnyWhere
| @@ -501,12 +501,13 @@ void main() { | |||
| ); | |||
| }); | |||
|
|
|||
| testWidgets('Top RefreshIndicator showed when dragging from non-zero scroll position', (WidgetTester tester) async { | |||
| testWidgets('Top RefreshIndicator(loose mode) should be shown when dragging from non-zero scroll position', (WidgetTester tester) async { | |||
There was a problem hiding this comment.
loose mode -> onEdge mode
| expect(find.byType(RefreshProgressIndicator), findsNothing); | ||
| }); | ||
|
|
||
| testWidgets('Top RefreshIndicator(strict mode) should not be shown when dragging from non-zero scroll position', (WidgetTester tester) async { |
Description
#71303 let the
RefreshIndicatorcan be pulled out from the non-zero scroll position no matter the overscroll occurs due to drag or inertia.This change cancel shows the
RefreshIndicatorwhen overscroll occurs due to inertia. I think this is intended to make it so that someone who is just scrolling to the top of your list doesn't accidentally start a refresh.After this change, The indicator will be pulled out in two cases,
// 1, Begin drag when the scrollable widget at the edge with zero scroll position.
// 2, Keep drag before overscroll occurs when the scrollable widget has
// a non-zero scroll position(do not release finger before overscroll).
This behavior is consistent with the Android native app(Test with Microsoft Outlook APP).
@Hixie suggested that this behavior can opt-in and tracked by #9690
Drag form edge
Keep drag when overscroll occurs from non-zero scroll position
Release finger before overscroll occurs(OverscrllIndicator is shown instead of RefreshIndicator)
Related Issues
Fixes #71936
Fixes #9690
Tests
I added the following tests:
See files.