Visitar URL original
Fix TabBarView/PageView edge auto-scroll during mouse text selection by mbcorona · Pull Request #189544 · flutter/flutter · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/flutter/lib/src/widgets/page_view.dart
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,11 @@ class PageScrollPhysics extends ScrollPhysics {
return PageScrollPhysics(parent: buildParent(ancestor));
}

// Selection gestures such as dragging to select text must not scroll a
// PageView to an adjacent page, so edge scrolling is disallowed here.
@override

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: consider adding a comment explaining why this is set to false for pageview physics. Something simple like Defaults to false to prevent selection gestures such as dragging to select text from scrolling to an adjacent page.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

bool get allowSelectionEdgeScrolling => false;

double _getPage(ScrollMetrics position) {
if (position is _PagePosition) {
return position.page!;
Expand Down
10 changes: 10 additions & 0 deletions packages/flutter/lib/src/widgets/scroll_physics.dart
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,16 @@ class ScrollPhysics {
/// Whether a viewport is allowed to change the scroll position as the result of user input.
bool get allowUserScrolling => true;

/// Whether a viewport may scroll to reveal more content when a selection
/// gesture extends past the visible area, i.e. the viewport's edge.
///
/// Paged scrollables that want to prevent selection gestures from revealing an
/// adjacent page should override this to false in their [ScrollPhysics],
/// similar to [PageScrollPhysics] used by [PageView].
///
/// Defaults to true, or to the value of the [parent] physics when there is one.
bool get allowSelectionEdgeScrolling => parent?.allowSelectionEdgeScrolling ?? true;

/// Called whenever a [Scrollable] is rebuilt with a new [ScrollPhysics]
/// of the same [runtimeType].
///
Expand Down
6 changes: 6 additions & 0 deletions packages/flutter/lib/src/widgets/scrollable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1301,6 +1301,12 @@ class _ScrollableSelectionContainerDelegate extends MultiSelectableSelectionCont
return result;
}
if (_selectionStartsInScrollable) {
// Paged scrollables (like PageView) disallow this; edge scrolling there
// flips to an adjacent page instead of revealing more content.
if (!(state.resolvedPhysics?.allowSelectionEdgeScrolling ?? true)) {
_autoScroller.stopAutoScroll();
return result;
}
_autoScroller.startAutoScrollIfNecessary(_dragTargetFromEvent(event));
if (_autoScroller.scrolling) {
return SelectionResult.pending;
Expand Down
10 changes: 10 additions & 0 deletions packages/flutter/test/widgets/scroll_physics_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ class TestScrollPhysics extends ScrollPhysics {
void main() {
const kBlueColor = Color(0xFF0000FF);

test('ScrollPhysics.allowSelectionEdgeScrolling', () {
expect(const BouncingScrollPhysics().allowSelectionEdgeScrolling, isTrue);
expect(const PageScrollPhysics().allowSelectionEdgeScrolling, isFalse);
// Resolves through the parent chain so wrapped page physics still count.
expect(
const ClampingScrollPhysics(parent: PageScrollPhysics()).allowSelectionEdgeScrolling,
isFalse,
);
});

test('ScrollPhysics applyTo()', () {
const a = TestScrollPhysics(name: 'a');
const b = TestScrollPhysics(name: 'b');
Expand Down
171 changes: 171 additions & 0 deletions packages/flutter/test/widgets/scrollable_selection_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,66 @@ Offset globalize(Offset point, RenderBox box) {
return box.localToGlobal(point);
}

// Drag-selects past the edge of a ListView nested in a PageView along [axis],
// and checks the ListView edge scrolls while the PageView stays on its page.
Future<void> selectPastNestedListEdge(WidgetTester tester, Axis axis) async {
final pageController = PageController();
addTearDown(pageController.dispose);
final listController = ScrollController();
addTearDown(listController.dispose);
await tester.pumpWidget(
TestWidgetsApp(
home: SelectableRegion(
selectionControls: testTextSelectionHandleControls,
child: PageView(
scrollDirection: axis,
controller: pageController,
children: <Widget>[
ListView.builder(
scrollDirection: axis,
controller: listController,
itemCount: 100,
itemBuilder: (BuildContext context, int index) {
return Center(child: Text('Item $index'));
},
),
const Center(child: Text('Second page')),
],
),
),
),
);
await tester.pumpAndSettle();
expect(pageController.page, 0.0);
expect(listController.offset, 0.0);

final RenderParagraph item0 = tester.renderObject<RenderParagraph>(
find.descendant(of: find.text('Item 0'), matching: find.byType(RichText)),
);
final Rect box = tester.getRect(find.byType(PageView));
final target = axis == Axis.vertical
? Offset(box.center.dx, box.bottom + 40.0)
: Offset(box.right + 40.0, box.center.dy);

final TestGesture gesture = await tester.startGesture(
textOffsetToPosition(item0, 2),
kind: PointerDeviceKind.mouse,
);
addTearDown(gesture.removePointer);
await tester.pump();
await gesture.moveTo(target);
await tester.pump();
await tester.pump(const Duration(seconds: 1));

// The inner ListView edge scrolled, but the PageView did not flip pages.
expect(listController.offset, greaterThan(0.0));
expect(pageController.page, 0.0);
expect(tester.takeException(), isNull);

await gesture.up();
await tester.pumpAndSettle();
}

void main() {
TestWidgetsFlutterBinding.ensureInitialized();
final mockClipboard = MockClipboard();
Expand Down Expand Up @@ -1499,6 +1559,117 @@ void main() {
expect(tester.takeException(), isNull);
});

testWidgets(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add an additional test that verifies a ListView inside a PageView can still be scrolled (while not flipping to an adjancent page), in both vertical and horizontal axes.

'paged scrollable does not flip pages during selection, but text still gets selected',
(WidgetTester tester) async {
final controller = PageController();
addTearDown(controller.dispose);
await tester.pumpWidget(
TestWidgetsApp(
home: SelectableRegion(
selectionControls: testTextSelectionHandleControls,
child: PageView.builder(
controller: controller,
itemCount: 5,
itemBuilder: (BuildContext context, int index) {
return Center(child: Text('Page $index'));
},
),
),
),
);
await tester.pumpAndSettle();
expect(controller.page, 0.0);

final RenderParagraph page0 = tester.renderObject<RenderParagraph>(
find.descendant(of: find.text('Page 0'), matching: find.byType(RichText)),
);
final TestGesture gesture = await tester.startGesture(
tester.getCenter(find.text('Page 0')),
kind: ui.PointerDeviceKind.mouse,
);
addTearDown(gesture.removePointer);
await tester.pump();
expect(controller.page, 0.0);

// Hold the drag past the right edge; without the fix this keeps flipping
// pages toward page 1.
await gesture.moveTo(tester.getBottomRight(find.byType(PageView)) + const Offset(40.0, 0.0));
await tester.pump();
await tester.pump(const Duration(seconds: 1));

// The page did not flip, but the page's text was still selected.
expect(controller.page, 0.0);
expect(page0.selections, isNotEmpty);
expect(tester.takeException(), isNull);

await gesture.up();
await tester.pumpAndSettle();
expect(controller.page, 0.0);
expect(tester.takeException(), isNull);
},
);

testWidgets('mouse selection drag still edge auto-scrolls a non page-snapping scrollable', (
WidgetTester tester,
) async {
// Counterpart to the PageView test above: only paged scrollables are
// affected. A plain ListView still edge scrolls while drag-selecting.
final controller = ScrollController();
addTearDown(controller.dispose);
await tester.pumpWidget(
TestWidgetsApp(
home: SelectableRegion(
selectionControls: testTextSelectionHandleControls,
child: ListView.builder(
controller: controller,
scrollDirection: Axis.horizontal,
itemCount: 100,
itemBuilder: (BuildContext context, int index) {
return Center(child: Text('Item $index'));
},
),
),
),
);
await tester.pumpAndSettle();
expect(controller.offset, 0.0);

final RenderParagraph paragraph0 = tester.renderObject<RenderParagraph>(
find.descendant(of: find.text('Item 0'), matching: find.byType(RichText)),
);
final TestGesture gesture = await tester.startGesture(
textOffsetToPosition(paragraph0, 2),
kind: ui.PointerDeviceKind.mouse,
);
addTearDown(gesture.removePointer);
await tester.pump();
expect(controller.offset, 0.0);

// Drag past the right edge to kick off auto-scroll.
await gesture.moveTo(tester.getBottomRight(find.byType(ListView)) + const Offset(40.0, 0.0));
await tester.pump();
await tester.pump(const Duration(seconds: 1));
expect(controller.offset, greaterThan(0.0));
expect(tester.takeException(), isNull);

await gesture.up();
await tester.pumpAndSettle();
expect(tester.takeException(), isNull);
});

testWidgets('vertical ListView inside a PageView still edge scrolls during selection', (
WidgetTester tester,
) async {
await selectPastNestedListEdge(tester, Axis.vertical);
});

testWidgets('horizontal ListView inside a PageView still edge scrolls during selection', (
WidgetTester tester,
) async {
await selectPastNestedListEdge(tester, Axis.horizontal);
});

group('Complex cases', () {
testWidgets('selection starts outside of the scrollable', (WidgetTester tester) async {
final controller = ScrollController();
Expand Down