Visitar URL original
Refactor comments for scrollable snapping behavior · flutter/flutter@b2041c7 · GitHub
Skip to content

Commit b2041c7

Browse files
mbcoronambcorona
committed
Refactor comments for scrollable snapping behavior
Updated comments to use Dart documentation style.
1 parent 0294ba7 commit b2041c7

7 files changed

Lines changed: 25 additions & 63 deletions

File tree

‎packages/flutter/lib/src/rendering/selection.dart‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@
66
/// @docImport 'package:flutter/material.dart';
77
library;
88

9-
import 'dart:ui' show PointerDeviceKind;
10-
119
import 'package:flutter/foundation.dart';
1210
import 'package:vector_math/vector_math_64.dart';
1311

@@ -522,14 +520,9 @@ class SelectionEdgeUpdateEvent extends SelectionEvent {
522520
///
523521
/// The [granularity] contains the granularity which the selection edge should move by.
524522
/// This value defaults to [TextGranularity.character].
525-
///
526-
/// The [deviceKind] is the pointer device that drove this update, if known,
527-
/// so edge auto-scrolling can respect the enclosing scrollable's drag
528-
/// devices. It is null when the driving device isn't known.
529523
const SelectionEdgeUpdateEvent.forStart({
530524
required this.globalPosition,
531525
TextGranularity? granularity,
532-
this.deviceKind,
533526
}) : granularity = granularity ?? TextGranularity.character,
534527
super._(SelectionEventType.startEdgeUpdate);
535528

@@ -539,14 +532,9 @@ class SelectionEdgeUpdateEvent extends SelectionEvent {
539532
///
540533
/// The [granularity] contains the granularity which the selection edge should move by.
541534
/// This value defaults to [TextGranularity.character].
542-
///
543-
/// The [deviceKind] is the pointer device that drove this update, if known,
544-
/// so edge auto-scrolling can respect the enclosing scrollable's drag
545-
/// devices. It is null when the driving device isn't known.
546535
const SelectionEdgeUpdateEvent.forEnd({
547536
required this.globalPosition,
548537
TextGranularity? granularity,
549-
this.deviceKind,
550538
}) : granularity = granularity ?? TextGranularity.character,
551539
super._(SelectionEventType.endEdgeUpdate);
552540

@@ -559,17 +547,6 @@ class SelectionEdgeUpdateEvent extends SelectionEvent {
559547
///
560548
/// Defaults to [TextGranularity.character].
561549
final TextGranularity granularity;
562-
563-
/// The pointer device that drove this selection edge update.
564-
///
565-
/// Edge auto-scrolling uses this to decide whether the enclosing scrollable
566-
/// should scroll while selecting: a paged scrollable ignores devices that
567-
/// aren't in its [ScrollBehavior.dragDevices].
568-
///
569-
/// This is null when the device isn't known, for example an event synthesized
570-
/// to keep a newly added selectable in sync. A null value leaves
571-
/// auto-scrolling untouched.
572-
final PointerDeviceKind? deviceKind;
573550
}
574551

575552
/// Extends the start or end of the selection by a given [TextGranularity].

‎packages/flutter/lib/src/widgets/page_view.dart‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,9 @@ class PageScrollPhysics extends ScrollPhysics {
573573
return PageScrollPhysics(parent: buildParent(ancestor));
574574
}
575575

576+
@override
577+
bool get snapsToPages => true;
578+
576579
double _getPage(ScrollMetrics position) {
577580
if (position is _PagePosition) {
578581
return position.page!;

‎packages/flutter/lib/src/widgets/scroll_physics.dart‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,13 @@ class ScrollPhysics {
499499
/// Whether a viewport is allowed to change the scroll position as the result of user input.
500500
bool get allowUserScrolling => true;
501501

502+
/// Whether this physics snaps to page boundaries, as [PageScrollPhysics] does.
503+
///
504+
/// Selection edge auto-scrolling checks this to avoid scrolling a paged
505+
/// scrollable (such as a [PageView] or [TabBarView]) while extending a
506+
/// selection, which would flip pages instead of revealing more content.
507+
bool get snapsToPages => parent?.snapsToPages ?? false;
508+
502509
@override
503510
String toString() {
504511
if (parent == null) {

‎packages/flutter/lib/src/widgets/scrollable.dart‎

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import 'framework.dart';
2828
import 'gesture_detector.dart';
2929
import 'media_query.dart';
3030
import 'notification_listener.dart';
31-
import 'page_view.dart';
3231
import 'restoration.dart';
3332
import 'restoration_properties.dart';
3433
import 'scroll_activity.dart';
@@ -1278,7 +1277,6 @@ class _ScrollableSelectionContainerDelegate extends MultiSelectableSelectionCont
12781277
event = SelectionEdgeUpdateEvent.forEnd(
12791278
globalPosition: endOffset,
12801279
granularity: event.granularity,
1281-
deviceKind: event.deviceKind,
12821280
);
12831281
} else {
12841282
_currentDragStartRelatedToOrigin = _inferPositionRelatedToOrigin(event.globalPosition);
@@ -1289,7 +1287,6 @@ class _ScrollableSelectionContainerDelegate extends MultiSelectableSelectionCont
12891287
event = SelectionEdgeUpdateEvent.forStart(
12901288
globalPosition: startOffset,
12911289
granularity: event.granularity,
1292-
deviceKind: event.deviceKind,
12931290
);
12941291
}
12951292
final SelectionResult result = super.handleSelectionEdgeUpdate(event);
@@ -1302,20 +1299,9 @@ class _ScrollableSelectionContainerDelegate extends MultiSelectableSelectionCont
13021299
return result;
13031300
}
13041301
if (_selectionStartsInScrollable) {
1305-
// Don't let a selection drag flip a PageView or TabBarView to the next
1306-
// page when it wouldn't accept a plain drag from this device (a mouse, on
1307-
// desktop and web). Otherwise the page animates over and snaps back mid
1308-
// selection. Other scrollables like ListView still auto scroll so mouse
1309-
// "select to scroll" keeps working. A null device kind means we don't
1310-
// know, so leave the behavior as it was.
1311-
//
1312-
// Note this is a different case from the shouldAcceptUserOffset check in
1313-
// EdgeDraggingAutoScroller; page physics accept user offset, so that
1314-
// check never catches this.
1315-
final PointerDeviceKind? deviceKind = event.deviceKind;
1316-
if (deviceKind != null &&
1317-
_scrollableSnapsToPages &&
1318-
!state._configuration.dragDevices.contains(deviceKind)) {
1302+
// A paged scrollable (PageView or TabBarView) would flip pages instead of
1303+
// revealing more content, so never edge scroll it while selecting.
1304+
if (state.resolvedPhysics?.snapsToPages ?? false) {
13191305
_autoScroller.stopAutoScroll();
13201306
return result;
13211307
}
@@ -1327,21 +1313,6 @@ class _ScrollableSelectionContainerDelegate extends MultiSelectableSelectionCont
13271313
return result;
13281314
}
13291315

1330-
// Whether this scrollable snaps to pages, like a PageView or TabBarView.
1331-
// Page physics can be wrapped by other physics, so check the whole chain.
1332-
bool get _scrollableSnapsToPages {
1333-
for (
1334-
ScrollPhysics? physics = state.resolvedPhysics;
1335-
physics != null;
1336-
physics = physics.parent
1337-
) {
1338-
if (physics is PageScrollPhysics) {
1339-
return true;
1340-
}
1341-
}
1342-
return false;
1343-
}
1344-
13451316
Offset _inferPositionRelatedToOrigin(Offset globalPosition) {
13461317
final box = state.context.findRenderObject()! as RenderBox;
13471318
final Offset localPosition = box.globalToLocal(globalPosition);

‎packages/flutter/lib/src/widgets/selectable_region.dart‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,7 +1122,6 @@ class SelectableRegionState extends State<SelectableRegion>
11221122
SelectionEdgeUpdateEvent.forEnd(
11231123
globalPosition: _selectionEndPosition!,
11241124
granularity: textGranularity,
1125-
deviceKind: _lastPointerDeviceKind,
11261125
),
11271126
) ==
11281127
SelectionResult.pending) {
@@ -1177,7 +1176,6 @@ class SelectableRegionState extends State<SelectableRegion>
11771176
SelectionEdgeUpdateEvent.forStart(
11781177
globalPosition: _selectionStartPosition!,
11791178
granularity: textGranularity,
1180-
deviceKind: _lastPointerDeviceKind,
11811179
),
11821180
) ==
11831181
SelectionResult.pending) {

‎packages/flutter/test/widgets/scroll_physics_test.dart‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,13 @@ class TestScrollPhysics extends ScrollPhysics {
3030
void main() {
3131
const kBlueColor = Color(0xFF0000FF);
3232

33+
test('ScrollPhysics.snapsToPages', () {
34+
expect(const BouncingScrollPhysics().snapsToPages, isFalse);
35+
expect(const PageScrollPhysics().snapsToPages, isTrue);
36+
// Propagates through the parent chain so wrapped page physics still count.
37+
expect(const ClampingScrollPhysics(parent: PageScrollPhysics()).snapsToPages, isTrue);
38+
});
39+
3340
test('ScrollPhysics applyTo()', () {
3441
const a = TestScrollPhysics(name: 'a');
3542
const b = TestScrollPhysics(name: 'b');

‎packages/flutter/test/widgets/scrollable_selection_test.dart‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1504,8 +1504,8 @@ void main() {
15041504
WidgetTester tester,
15051505
) async {
15061506
// Regression test for https://github.com/flutter/flutter/issues/149426.
1507-
// A mouse isn't one of a PageView's default drag devices, so dragging a
1508-
// selection past its edge shouldn't flip to the next page and snap back.
1507+
// A paged scrollable snaps to pages, so dragging a selection past its edge
1508+
// must not flip to the next page and snap back.
15091509
final controller = PageController();
15101510
addTearDown(controller.dispose);
15111511
await tester.pumpWidget(
@@ -1553,10 +1553,9 @@ void main() {
15531553
testWidgets('mouse selection drag still edge auto-scrolls a non page-snapping scrollable', (
15541554
WidgetTester tester,
15551555
) async {
1556-
// The counterpart to the PageView test above: the fix only touches paged
1557-
// scrollables. A plain ListView still scrolls while you drag-select with a
1558-
// mouse ("select to scroll"), even though a mouse isn't one of its drag
1559-
// devices. See https://github.com/flutter/flutter/issues/149426.
1556+
// Counterpart to the PageView test above: the guard only touches paged
1557+
// scrollables. A plain ListView still edge scrolls while drag-selecting
1558+
// ("select to scroll"). See https://github.com/flutter/flutter/issues/149426.
15601559
final controller = ScrollController();
15611560
addTearDown(controller.dispose);
15621561
await tester.pumpWidget(

0 commit comments

Comments
 (0)