Correct text selection pivot points#71756
Conversation
…orrects pivot points.
dd3f50b to
967a9fa
Compare
e2544ec to
3a304d5
Compare
|
Reviewers: Can you think of any reason why we were swapping base/extent before? This seems to work better without the swap and doesn't seem to break major tests, but I'm keeping an eye out. |
| baseOffset = math.min(fromPosition.offset, toPosition.offset); | ||
| extentOffset = math.max(fromPosition.offset, toPosition.offset); | ||
| } | ||
| final int baseOffset = fromPosition.offset; |
There was a problem hiding this comment.
This seems to revert the changes made in #29395. According to @mdebbar
The problem is if you drag from left to right, the text will be selected as expected. But if you drag from right to left, then baseOffset will be greater than extentOffset and it'll be collapsed down to a cursor. So dragging from right to left is broken, and this PR fixes it.
Not sure why the tests are passing. But it sounds like we're not handling baseOffset > extentOffset correctly?
There was a problem hiding this comment.
Ah thanks for tracking that PR down. It seems like the bug that that fixed, where the cursor would collapsed when selecting right to left, no longer happens. Everything still seems to be fine with this PR here by my testing.
I should point out that we're still reversing base and extent in the underlying TextRange. That seems to have been around for a long time, though.
flutter/packages/flutter/lib/src/services/text_editing.dart
Lines 18 to 26 in 72aa23e
| extentOffset = math.max(fromPosition.offset, toPosition.offset); | ||
| } | ||
| final int baseOffset = fromPosition.offset; | ||
| final int extentOffset = toPosition == null ? fromPosition.offset : toPosition.offset; |
There was a problem hiding this comment.
nit: from the previous code it could be toPosition?.offset ?? fromPosition.offset
Description
The pivot point is the selection edge that does not move:
Currently, Flutter always puts the pivot point on the right, but as you can see in the gifs above of native OSX, other platforms put it where the selection began. This PR updates Flutter to do the same.
There are more complicated platform-specific behaviors in other selection scenarios, such as when using word and line modifier keys, but that isn't handled by this PR. See #57679 (comment).
Related Issues
Closes #57679
Tests
I added the following tests: