-
Notifications
You must be signed in to change notification settings - Fork 384
fix(ui): jump on text selection #2864
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
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
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
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
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
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
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
232 changes: 232 additions & 0 deletions
232
packages/stream_chat_flutter/test/scrollable_positioned_list/reveal_anchor_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,232 @@ | ||
| // Tests for anchor-aware `getOffsetToReveal` on the SPL viewports. | ||
| // | ||
| // `RenderViewportBase.getOffsetToReveal` resolves a target into the | ||
| // viewport's scroll-offset space and hands that value back as the | ||
| // `offset.pixels` to move to. That conversion assumes scroll offset 0 | ||
| // sits at the viewport's leading edge — true only when `anchor` is 0. | ||
| // The SPL viewports place it at `mainAxisExtent * anchor - pixels`, and | ||
| // their `anchor` is deliberately unbounded (anchor preservation folds | ||
| // accumulated scroll pixels into it as the list paginates). | ||
| // | ||
| // Anything that reveals a descendant therefore used to jump the list by | ||
| // `anchor * mainAxisExtent`. The user-visible symptom was a channel | ||
| // teleporting several screens when a message's selectable text moved the | ||
| // selection and `RenderEditable.showOnScreen` fired. | ||
| // | ||
| // See https://github.com/GetStream/stream-chat-flutter/issues/2862. | ||
|
|
||
| import 'package:flutter/foundation.dart'; | ||
| import 'package:flutter/gestures.dart'; | ||
| import 'package:flutter/material.dart'; | ||
| import 'package:flutter_test/flutter_test.dart'; | ||
| import 'package:stream_chat_flutter/scrollable_positioned_list/scrollable_positioned_list.dart'; | ||
| // Not exported from the barrel file; imported directly to assert which | ||
| // viewport implementation `shrinkWrap: true` actually builds. | ||
| import 'package:stream_chat_flutter/scrollable_positioned_list/src/wrapping.dart'; | ||
|
|
||
| const _viewportHeight = 600.0; | ||
| const _viewportWidth = 400.0; | ||
| const _itemHeight = 40.0; | ||
| const _itemCount = 500; | ||
| const _positionedIndex = 100; | ||
|
|
||
| /// A large, out-of-`[0, 1]` anchor — the state the list reaches once anchor | ||
| /// preservation has folded scroll pixels into the alignment a few times. | ||
| const _anchor = 3.0; | ||
|
|
||
| void main() { | ||
| Future<void> pump( | ||
| WidgetTester tester, { | ||
| required bool reverse, | ||
| bool selectableItems = false, | ||
| bool shrinkWrap = false, | ||
| }) async { | ||
| tester.view.devicePixelRatio = 1.0; | ||
| tester.view.physicalSize = const Size(_viewportWidth, _viewportHeight); | ||
| addTearDown(tester.view.resetPhysicalSize); | ||
| addTearDown(tester.view.resetDevicePixelRatio); | ||
|
|
||
| final controller = ItemScrollController(); | ||
| await tester.pumpWidget( | ||
| MaterialApp( | ||
| home: Scaffold( | ||
| body: ScrollablePositionedList.builder( | ||
| itemCount: _itemCount, | ||
| reverse: reverse, | ||
| shrinkWrap: shrinkWrap, | ||
| itemScrollController: controller, | ||
| itemBuilder: (context, i) => SizedBox( | ||
| key: ValueKey('item-$i'), | ||
| height: _itemHeight, | ||
| child: selectableItems ? SelectableText('item-$i') : Text('item-$i'), | ||
| ), | ||
| ), | ||
| ), | ||
| ), | ||
| ); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| controller.jumpTo(index: _positionedIndex, alignment: _anchor); | ||
| await tester.pumpAndSettle(); | ||
| } | ||
|
|
||
| ScrollPosition positionOf(WidgetTester tester) => | ||
| tester.state<ScrollableState>(find.byType(Scrollable).first).position; | ||
|
|
||
| /// Distance from the viewport's leading edge to [item]'s leading edge, | ||
| /// along the scroll axis. | ||
| double leadingEdgeOf(WidgetTester tester, Finder item, {required bool reverse}) { | ||
| final rect = tester.getRect(item); | ||
| return reverse ? _viewportHeight - rect.bottom : rect.top; | ||
| } | ||
|
|
||
| /// An on-screen item that is a few rows in from the leading edge, so a | ||
| /// reveal is expected to scroll by a known, non-zero amount. | ||
| Finder itemInsideViewport(WidgetTester tester, {required bool reverse}) { | ||
| final candidates = | ||
| tester | ||
| .widgetList<SizedBox>(find.byType(SizedBox)) | ||
| .map((w) => w.key) | ||
| .whereType<ValueKey<String>>() | ||
| .map(find.byKey) | ||
| .where((f) { | ||
| final edge = leadingEdgeOf(tester, f, reverse: reverse); | ||
| return edge > 0 && edge < _viewportHeight - _itemHeight; | ||
| }) | ||
| .toList() | ||
| ..sort( | ||
| (a, b) => leadingEdgeOf(tester, a, reverse: reverse).compareTo(leadingEdgeOf(tester, b, reverse: reverse)), | ||
| ); | ||
| expect(candidates, isNotEmpty, reason: 'need an on-screen item to reveal'); | ||
| return candidates[candidates.length ~/ 2]; | ||
| } | ||
|
|
||
| for (final reverse in [false, true]) { | ||
| group('reverse: $reverse', () { | ||
| testWidgets('ensureVisible moves by exactly the on-screen offset', (tester) async { | ||
| await pump(tester, reverse: reverse); | ||
|
|
||
| final target = itemInsideViewport(tester, reverse: reverse); | ||
| final expectedDelta = leadingEdgeOf(tester, target, reverse: reverse); | ||
| final before = positionOf(tester).pixels; | ||
|
|
||
| await Scrollable.ensureVisible(tester.element(target)); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| expect( | ||
| positionOf(tester).pixels - before, | ||
| // `pixels` grows along the axis direction, so revealing an item | ||
| // that sits `expectedDelta` past the leading edge always moves it | ||
| // forward by that much — regardless of `reverse`. | ||
| closeTo(expectedDelta, 1), | ||
| reason: | ||
| 'reveal must not add anchor * viewportDimension ' | ||
| '(${_anchor * _viewportHeight}px) to the scroll offset', | ||
| ); | ||
| expect( | ||
| leadingEdgeOf(tester, target, reverse: reverse), | ||
| closeTo(0, 1), | ||
| reason: 'the revealed item should sit at the leading edge', | ||
| ); | ||
| }); | ||
|
|
||
| testWidgets('a second ensureVisible on the same item is a no-op', (tester) async { | ||
| await pump(tester, reverse: reverse); | ||
|
|
||
| final target = itemInsideViewport(tester, reverse: reverse); | ||
| await Scrollable.ensureVisible(tester.element(target)); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| final settled = positionOf(tester).pixels; | ||
| await Scrollable.ensureVisible(tester.element(target)); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| expect(positionOf(tester).pixels, closeTo(settled, 1)); | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| // `shrinkWrap: true` swaps `UnboundedViewport` for | ||
| // `CustomShrinkWrappingViewport`, so the reveal correction runs through | ||
| // `CustomRenderShrinkWrappingViewport.getOffsetToReveal` — a separate | ||
| // implementation from the one the groups above cover. | ||
| group('shrinkWrap', () { | ||
| testWidgets('the shrink-wrapping viewport is the one under test', (tester) async { | ||
| await pump(tester, reverse: true, shrinkWrap: true); | ||
|
|
||
| expect( | ||
| tester.allRenderObjects.whereType<CustomRenderShrinkWrappingViewport>(), | ||
| isNotEmpty, | ||
| reason: 'shrinkWrap: true must build the shrink-wrapping viewport', | ||
| ); | ||
| }); | ||
|
|
||
| for (final reverse in [false, true]) { | ||
| testWidgets('ensureVisible moves by exactly the on-screen offset (reverse: $reverse)', (tester) async { | ||
| await pump(tester, reverse: reverse, shrinkWrap: true); | ||
|
|
||
| final target = itemInsideViewport(tester, reverse: reverse); | ||
| final expectedDelta = leadingEdgeOf(tester, target, reverse: reverse); | ||
| final before = positionOf(tester).pixels; | ||
|
|
||
| await Scrollable.ensureVisible(tester.element(target)); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| expect( | ||
| positionOf(tester).pixels - before, | ||
| closeTo(expectedDelta, 1), | ||
| reason: | ||
| 'reveal must not add anchor * mainAxisExtent ' | ||
| '(${_anchor * _viewportHeight}px) to the scroll offset', | ||
| ); | ||
| expect( | ||
| leadingEdgeOf(tester, target, reverse: reverse), | ||
| closeTo(0, 1), | ||
| reason: 'the revealed item should sit at the leading edge', | ||
| ); | ||
| }); | ||
|
|
||
| testWidgets('a second ensureVisible on the same item is a no-op (reverse: $reverse)', (tester) async { | ||
| await pump(tester, reverse: reverse, shrinkWrap: true); | ||
|
|
||
| final target = itemInsideViewport(tester, reverse: reverse); | ||
| await Scrollable.ensureVisible(tester.element(target)); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| final settled = positionOf(tester).pixels; | ||
| await Scrollable.ensureVisible(tester.element(target)); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| expect(positionOf(tester).pixels, closeTo(settled, 1)); | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| testWidgets('selecting text in a message does not scroll the list', (tester) async { | ||
| // Reset inside the body rather than via `addTearDown`: `flutter_test` | ||
| // asserts the foundation debug vars are unset at the end of the test | ||
| // body, which runs before any tear-down. | ||
| debugDefaultTargetPlatformOverride = TargetPlatform.macOS; | ||
| try { | ||
| await pump(tester, reverse: true, selectableItems: true); | ||
|
|
||
| final target = itemInsideViewport(tester, reverse: true); | ||
| final before = positionOf(tester).pixels; | ||
|
|
||
| // Drag-select across the text. On macOS this reports | ||
| // `SelectionChangedCause.drag`, which makes `EditableText` call | ||
| // `bringIntoView` -> `RenderEditable.showOnScreen`. | ||
| final start = tester.getCenter(target) - const Offset(30, 0); | ||
| final gesture = await tester.startGesture(start, kind: PointerDeviceKind.mouse); | ||
| await tester.pump(const Duration(milliseconds: 50)); | ||
| await gesture.moveTo(start + const Offset(50, 0)); | ||
| await tester.pumpAndSettle(); | ||
| await gesture.up(); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| expect(positionOf(tester).pixels, closeTo(before, 1)); | ||
| } finally { | ||
| debugDefaultTargetPlatformOverride = null; | ||
| } | ||
| }); | ||
| } |
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.
Uh oh!
There was an error while loading. Please reload this page.