Fix CommandPalette to prefer inner interactions over bindings #9056
Conversation
ghost
added
Area-CmdPal
|
I should stop creating conflicts for myself |
ghost
requested review from
miniksa,
carlos-zamora,
DHowett,
leonMSFT and
PankajBhojwani
| else if (key == VirtualKey::C && ctrlDown) | ||
| { | ||
| _searchBox().CopySelectionToClipboard(); | ||
| e.Handled(true); | ||
| } | ||
| else if (key == VirtualKey::V && ctrlDown) | ||
| { | ||
| _searchBox().PasteFromClipboard(); | ||
| e.Handled(true); | ||
| } |
There was a problem hiding this comment.
Should we also hardcode some other text box default bindings like ctrl+a, ctrl+left/right, ctrl+x, and home/end? Or what makes ctrl+c and ctrl+v so special here?
There was a problem hiding this comment.
Isn't there a way to ask the text box to handle the event first, and if it can't then we handle it? Or how does the order of handling work?
There was a problem hiding this comment.
Yes. Probably. I handled copy and paste explicitly as right now it is the same default binding for pasting in the terminal. Which means that the current behavior upon ctrl+v is to paste in the terminal.
| @@ -423,7 +422,6 @@ the MIT License. See LICENSE in the project root for license information. --> | |||
| AllowDrop="False" | |||
| IsItemClickEnabled="True" | |||
| ItemClick="_listItemClicked" | |||
| PreviewKeyDown="_keyDownHandler" | |||
There was a problem hiding this comment.
Just double-checking. So if we have focus on a list view item, any key stroke we make will go to the UserControl's PreviewKeyDown handler on line 18? And that'll properly handle navigating the list view with the arrow keys and home/end?
There was a problem hiding this comment.
Yes. Actually Preview is going top down... and if the key is handled by the _keyDownHandler the propagation stops. In other words, this line was doing nothing functional (because if the handler handles the event, the propagation would stop on the palette level, and if the handler does not - there is no need to propagate) 😄
ghost
added
Needs-Author-Feedback
|
Aaah you conflicted yourself |
I don't like it either. The main blocker is that ctrl+ events do not bubble up with KeyDown. This simply drives me mad. |
Story of my life. |
Indeed! They are "accelerators" 😄 which is a separate part of Xaml used to make keyboard shortcuts work. |
|
Resolved the conflicts 💪 |
* Currently TerminalPage registers on CmdPal key events:
* To invoke bindings when the palette is open
* Since some key combinations are not triggered by KeyDown
it registers for PreviewKeyDown
* As a result bindings might be preferred over navigation
(e.g., ctrl+v will paste into Terminal rather than into search box)
* To fix this, I moved all interactions inside the CmdPal into
PreviewKeyDown as well
* In addition, added specific handling for copy/paste
which now allow to interact with search box even if not focused
Closes #9044
(cherry picked from commit ed19301)
|
🎉 Handy links: |
PR Checklist
Detailed Description of the Pull Request / Additional comments
it registers for PreviewKeyDown
(e.g., ctrl+v will paste into Terminal rather than into search box)
PreviewKeyDown as well
which now allow to interact with search box even if not focused