[1.14] Fix keyboard selection and copyOnSelect interaction #13360
Conversation
|
@msftbot make sure @DHowett signs off on this |
ghost
added
the
AutoMerge
|
Hello @carlos-zamora! Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:
If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you". |
| // 2. right click always pastes! | ||
| if (_core->IsInQuickEditMode()) | ||
| { | ||
| CopySelectionToClipboard(shiftEnabled, nullptr); |
There was a problem hiding this comment.
If we're CopyOnSelect() == true wouldn't we have already copied the selection at this point in ControlInteractivity::PointerReleased?
When is IsInQuickEditMode() == false? Whenever we do a keyboard selection? Would IsInMouseSelectionMode (or inversely IsInKeyboardSelectionMode) not be a better (more descriptive and modern) function name then?
There was a problem hiding this comment.
If we're
CopyOnSelect() == truewouldn't we have already copied the selection at this point inControlInteractivity::PointerReleased?
Yes and no.
Yes, because that covers the scenario of (1) create a mouse selection then (2) releasing the mouse button. The content immediately gets copied to your clipboard. When you right-click, you paste that content, so this code isn't called at all because we're not in "quick edit mode".
No, because "quick edit mode" is specifically when you add these steps to the scenario above... (3) shift+right to extend the selection a bit. Then, when you right-click, we already copied the wrong contents! We copied the data when you let go, not when you're right-clicking. So this extra code just detects when you've done step 3 and we need to copy the contents again.
When is
IsInQuickEditMode() == false? Whenever we do a keyboard selection? WouldIsInMouseSelectionMode(or inverselyIsInKeyboardSelectionMode) not be a better (more descriptive and modern) function name then?
QuickEditMode == true --> you made a selection with the mouse, then modified it with the keyboard
MarkMode == true --> you toggled mark mode manually (created a selection at the cursor)
They're mutually exclusive. After a quick chat with Dustin, here's a new approach...
enum class SelectionMode
{
Mouse = 0,
Keyboard,
Mark
};
We just have 3 modes in increasing order. All mutually exclusive, but all useful to know when to show the selection markers and what trickery could occur. That also lets me combine IsInMarkMode and IsInQuickEditMode. AND it also gets rid of the historical term of "quick edit mode".
There was a problem hiding this comment.
this approach only applies to 1.15+
| // 2. right click always pastes! | ||
| if (_core->IsInQuickEditMode()) | ||
| { | ||
| CopySelectionToClipboard(shiftEnabled, nullptr); |
There was a problem hiding this comment.
this approach only applies to 1.15+
|
Apologies, while this PR appears ready to be merged, it looks like |
2 similar comments
|
Apologies, while this PR appears ready to be merged, it looks like |
|
Apologies, while this PR appears ready to be merged, it looks like |
|
🎉 Handy links: |
Summary of the Pull Request
Backport of #13358
This PR fixes a number of interactions between
copyOnSelectand keyboard selection. Sometimes the selection just wouldn't be cleared or copied appropriately.I wrote a whole flowchart of expected behavior with copy on select:
copyOnSelectcopyaction should clear the selection