Scrollbar thumb drag gestures now produce one start and one end scroll notification #146654
Conversation
github-actions
bot
added
framework
HansMuller
force-pushed
the
refactor_scrollbar
branch
from
d6ac513 to
05ea3dc
Compare
HansMuller
changed the title
HansMuller
force-pushed
the
refactor_scrollbar
branch
2 times, most recently
from
5f87208 to
1091d36
Compare
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
Overall I think this looks awesome, just a couple of questions
There was a problem hiding this comment.
For this and the other sample, why are these post frame callbacks?
There was a problem hiding this comment.
Because when the notification arrives the ScrollPosition is still in its DragScrollActivity. So calls to animateTo are ignored per https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/widgets/scroll_position.dart#L995
There was a problem hiding this comment.
Hmm, this makes me wonder if folks that wanted to do this would be better off listening to the isScrollingNotifier on the position? I think when it changes to false it is safe to alter the scroll position at that point.
Doing so this way feels kind of advanced for users, and should also account for whether or not the context is still mounted in that callback.
There was a problem hiding this comment.
The examples/api/lib/widgets/scroll_position/is_scrolling_listener.0.dart (included in this PR) does exactly that. This example is for ScrollEndNotification whose API doc does mention the isScrolling version of the example:
/// ... This sort of thing can also
/// be done by listening to the [ScrollController]'s
/// [ScrollPosition.isScrollingNotifier]. An alternative example is provided
/// with [ScrollPosition.isScrollingNotifier].
examples/api/lib/widgets/scroll_position/is_scrolling_listener.0.dart
Outdated
Show resolved
Hide resolved
HansMuller
force-pushed
the
refactor_scrollbar
branch
from
1091d36 to
a9ac207
Compare
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
LGTM, make it so. Nitty nit though on the sample code, WDYT?
There was a problem hiding this comment.
Hmm, this makes me wonder if folks that wanted to do this would be better off listening to the isScrollingNotifier on the position? I think when it changes to false it is safe to alter the scroll position at that point.
Doing so this way feels kind of advanced for users, and should also account for whether or not the context is still mounted in that callback.
|
HansMuller
force-pushed
the
refactor_scrollbar
branch
from
72de709 to
f13354c
Compare
HansMuller
added
the
autosubmit
…nd scroll notification (flutter/flutter#146654)
…nd scroll notification (flutter/flutter#146654)
…nd scroll notification (flutter/flutter#146654)
…nd scroll notification (flutter/flutter#146654)
…nd scroll notification (flutter/flutter#146654)
flutter/flutter@c246ecd...27e0656 2024-06-05 98614782+auto-submit[bot]@users.noreply.github.com Reverts "TreeSliver & associated classes (#147171)" (flutter/flutter#149754) 2024-06-05 [email protected] Feature: Add AnimatedList with separators (flutter/flutter#144899) 2024-06-05 [email protected] make output of flutter run web tests verbose (flutter/flutter#149694) 2024-06-05 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Request focus if `SemanticsAction.focus` is sent to a focusable widget (#142942)" (flutter/flutter#149741) 2024-06-05 [email protected] Roll Flutter Engine from e88469090fed to 11a32d43e3f6 (2 revisions) (flutter/flutter#149699) 2024-06-05 [email protected] Request focus if `SemanticsAction.focus` is sent to a focusable widget (flutter/flutter#142942) 2024-06-04 [email protected] Roll Flutter Engine from 3dd40156afb6 to e88469090fed (2 revisions) (flutter/flutter#149695) 2024-06-04 [email protected] TreeSliver & associated classes (flutter/flutter#147171) 2024-06-04 [email protected] Roll Flutter Engine from fe00f023666c to 3dd40156afb6 (3 revisions) (flutter/flutter#149692) 2024-06-04 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.7 to 3.25.8 (flutter/flutter#149691) 2024-06-04 [email protected] Prepares semantics_update_test for upcoming heading level changes (flutter/flutter#149671) 2024-06-04 [email protected] Identify and re-throw our dependency checking errors in `flutter.groovy` (flutter/flutter#149609) 2024-06-04 [email protected] Scrollbar thumb drag gestures now produce one start and one end scroll notification (flutter/flutter#146654) 2024-06-04 [email protected] Disable sandboxing for macOS apps and tests in CI (flutter/flutter#149618) 2024-06-04 [email protected] Roll Flutter Engine from e211c43f3dc1 to fe00f023666c (3 revisions) (flutter/flutter#149680) 2024-06-04 [email protected] Allow `find.byTooltip` to use a RegEx (flutter/flutter#149348) 2024-06-04 [email protected] Roll Flutter Engine from a6aa5d826649 to e211c43f3dc1 (8 revisions) (flutter/flutter#149658) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
|
This PR seems cause a regression issue when there is a nested ListViews:) Here is the sample code to test. In the demo, I'm trying to scroll to the right, but it doesn't work all the time and vertical thumb bar looks highlighted even when I scroll horizontally. Screen.Recording.2024-06-13.at.4.00.09.PM.movDetailsimport 'package:flutter/material.dart';
void main() => runApp(const CarouselExampleApp());
class CarouselExampleApp extends StatelessWidget {
const CarouselExampleApp({super.key});
@override
Widget build(BuildContext context) {
return MaterialApp(
debugShowCheckedModeBanner: false,
home: Scaffold(
appBar: AppBar(
title: const Text('Sample'),
),
body: const CarouselExample(),
),
);
}
}
class CarouselExample extends StatefulWidget {
const CarouselExample({super.key});
@override
State<CarouselExample> createState() => _CarouselExampleState();
}
class _CarouselExampleState extends State<CarouselExample> {
@override
Widget build(BuildContext context) {
return ListView(
children: <Widget>[
Container(
width: 200,
height: 200,
color: Colors.amber[50],
),
Center(
child: ConstrainedBox(
constraints: const BoxConstraints(maxHeight: 200),
child: ListView(
scrollDirection: Axis.horizontal,
children: List.generate(20, (int index) {
return ConstrainedBox(constraints: BoxConstraints.tightFor(width: 200), child: Card.outlined(child: SizedBox(width: 200, height: 200, child: Center(child: Text('Item $index')),),),);
}),
)
),
),
Container(
width: 200,
height: 200,
color: Colors.amber[100],
),
Container(
width: 200,
height: 200,
color: Colors.amber[200],
),
],
);
}
}CC: @Piinks |
|
@QuncCccccc can you put that in a new issue and cc @HansMuller? |
…pad (flutter#150275) Corrects some problems related to trackpad scrolls introduced by flutter#146654. Fixes flutter#149999 Fixes flutter#150342 Fixes flutter#150236
…nd scroll notification (flutter/flutter#146654)
3 participants
The scroll notification events reported for a press-drag-release gesture within a scrollable on a touch screen device begin with a
ScrollStartNotification, followed by a series ofScrollUpdateNotifications, and conclude with aScrollEndNotification. This protocol can be used to defer work until an interactive scroll gesture ends. For example, you might defer updating a scrollable's contents via network requests until the scroll has ended, or you might want to automatically auto-scroll at that time.In the example that follows the CustomScrollView automatically scrolls so that the last partially visible fixed-height item is completely visible when the scroll gesture ends. Many iOS applications do this kind of thing. It only makes sense to auto-scroll when the user isn't actively dragging the scrollable around.
It's easy enough to do this by reacting to a ScrollEndNotifcation by auto-scrolling to align the last fixed-height list item (source code).
Screen.Recording.2024-02-05.at.4.30.46.PM.mov
Dragging the scrollbar thumb in a desktop application is a similar user gesture. Currently it's not possible to defer work or auto-scroll (or whatever) while the scrollable is actively being dragged via the scrollbar thumb because each scrollbar thumb motion is mapped to a scroll start - scroll update - scroll end series of notifications. On a desktop platform, the same code behaves quite differently when the scrollbar thumb is dragged.
Screen.Recording.2024-02-05.at.9.45.41.AM.mov
The stream of scroll-end events triggers auto-scrolling every time the thumb moves. From the user's perspective this feels like a losing struggle.
One can also detect the beginning and end of a touch-drag by listening to the value of a ScrollPosition's
isScrollingNotifier. This approach suffers from a similar problem: during a scrollbar thumb-drag, theisScrollingNotifiervalue isn't updated at all.This PR refactors the RawScrollbar implementation to effectively use a ScrollDragController to manage scrolls caused by dragging the scrollbar's thumb. Doing so means that dragging the thumb will produce the same notifications as dragging the scrollable on a touch device.
Now desktop applications can choose to respond to scrollbar thumb drags in the same that they respond to drag scrolling on a touch screen. With the changes included here, the desktop or web version of the app works as expected, whether you're listing to scroll notifications or the scroll position's
isScrollingNotifier.Screen.Recording.2024-02-02.at.4.04.51.PM.mov
This PR also makes the second ScrollPosition API doc example work as expected when used with the DartPad that's part of API doc page.
Desktop applications also see scroll start-update-end notifications due to the mouse wheel. There is no touch screen analog for the mouse wheel, so an application that wanted to enable this kind of auto-scrolling alignment would have to include a heuristic that dealt with the sequence of small scrolls triggered by the mouse wheel. Here's an example of that: source code. This version of the app does not auto-align in response to small changes, wether they're triggered by dragging the scrollbar thumb of the mouse wheel.
Related sliver utility PRs: #143538, #143196, #143325.
Part of #127334.