fix(ui): jump on text selection - #2864
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds anchor-aware viewport reveal corrections, removes duplicate anchor adjustments from ChangesAnchor-aware reveal behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SelectableText
participant ScrollablePositionedList
participant AnchoredViewport
SelectableText->>ScrollablePositionedList: request showOnScreen
ScrollablePositionedList->>AnchoredViewport: request getOffsetToReveal
AnchoredViewport-->>ScrollablePositionedList: return corrected offset and rectangle
ScrollablePositionedList-->>SelectableText: preserve the target reveal position
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/stream_chat_flutter/lib/scrollable_positioned_list/src/wrapping.dart`:
- Around line 159-170: Update the reveal correction in getOffsetToReveal to use
the layout main-axis extent supplied to _attemptLayout rather than size,
especially for shrinkWrap layouts. Store that _attemptLayout extent on the
relevant state/object and use it for the anchor-based offset and rect
adjustment, preserving the existing finite-offset and zero-correction guards.
In
`@packages/stream_chat_flutter/test/scrollable_positioned_list/reveal_anchor_test.dart`:
- Around line 145-163: Register teardown to restore
debugDefaultTargetPlatformOverride before setting it to TargetPlatform.macOS in
this test, ensuring cleanup runs if pump, gesture operations, or the expectation
throws; remove reliance on the manual reset at the test body’s end.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3cc7e448-f9c6-4ad0-b67e-75920312ab00
📒 Files selected for processing (5)
packages/stream_chat_flutter/CHANGELOG.mdpackages/stream_chat_flutter/lib/scrollable_positioned_list/src/positioned_list.dartpackages/stream_chat_flutter/lib/scrollable_positioned_list/src/viewport.dartpackages/stream_chat_flutter/lib/scrollable_positioned_list/src/wrapping.dartpackages/stream_chat_flutter/test/scrollable_positioned_list/reveal_anchor_test.dart
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2864 +/- ##
==========================================
+ Coverage 72.86% 72.87% +0.01%
==========================================
Files 429 429
Lines 27698 27716 +18
==========================================
+ Hits 20183 20199 +16
- Misses 7515 7517 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Submit a pull request
Github Issue: fixes #2862
CLA
Description of the pull request
This PR fixes the specific case of text selection, but we might want a better fix.
Summary by CodeRabbit
Bug Fixes
StreamMessageListViewto jump multiple screens.Tests