Enable shortcut while CommandPalette is open

[ghost]

Summary of the Pull Request

It is maybe not the best way since I had to get all the cases for key handling so I just created for each of them. As a result the code get longer(not optimized). Most difficult thing was Next tab and Previous tab I just could not solve it.

9 commands that couldn't enabled > <

 1. Increase font size               -> could not find VirtualKey for "-"
 2. Decrease font size             -> could not find VirtualKey for "+"
 3. Split pane, split:horizontal -> could not find VirtualKey for "-"
 4. Split pane, split:vertical     -> could not find VirtualKey for "+"
 5. Open default settings        -> could not find VirtualKey for ","
 6. Open settings file               -> could not find VirtualKey for ","
 7. Open new tab dropdown    -> could not resolve keybindings
 8. Next tab                               -> could not resolve keybindings
 9. Previous tab                        -> could not resolve keybindings

References

PR Checklist

• Closes commandPalette keybinding should close the palette when it's open #6679
• CLA signed. If not, go over here and sign the CLA
• Tests added/passed
• Documentation updated. If checked, please file a pull request on our docs repo and link it here: #xxx
• Schema updated.
• I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

[zadjii-msft]

Okay this absolutely won't work - what if the user's re-defined their keybindings? What if they've added actions? As soon as the user's list of actions doesn't exactly match the defaults, this solution won't work.

A better idea would be to pass the key event to the IKeyBindings from the settings, and let that object try and figure out what the action for the given key chord would be.

[ghost]

I will try 🙃

[ghost]

I'm trying to get default.json using CascadiaSettings::LoadDefaults() and do KeyMap(), but it dosen't work.

[zadjii-msft]

I'm trying to get default.json using CascadiaSettings::LoadDefaults() and do KeyMap(), but it dosen't work.

I wouldn't bother with all that - the TerminalPage already has a perfectly good CascadiaSettings object (_settings) that's got the entirely loaded settings. Don't bother re-loading the settings, just use that one.

[zadjii-msft]

This works pretty well actually! There's only two cases that seem weird, and I'm not really sure if they're worth blocking this whole PR over:

• If you press ctrl+shift+p to dismiss the command palette, then immediately the palette will re-open itself on the key up. Maybe we need to special-case that action, to only dispatch it on a key-up?
• ctrl+tab to move to the next tab doesn't actually dispatch that action. It seems like the text box eats that keystroke first. That's probably okay though? At least intuitively, I saw that happen and thought "oh, yea that makes sense".

So I'm blocking over the ToggleCommandPalette action thing, but I am open to being convinced otherwise by other reviewers

[ghost]

@zadjii-msft I fixed second issue, but I couldn't understand the first problem. When I press ctrl+shift+p the cursor blinks that's it(is this because of re-opening?), I couldn't even dismiss the command palette. Dose this also occur for you as well?

[zadjii-msft]

Yea, so that's basically what's happening to me to. I'm guessing that what's happening is that all in the course of 1-2 frames, the palette is being dismissed, the focus is moving back to the control (causing the cursor to blink), and then on the key up of P, we're sending another toggleCommandPalette action, which is causing the palette to re-appear.

I don't have the branch checked out locally right now to confirm, but I bet if you press and hold the P key, you won't see the palette re-appear until the key is released.

[ghost]

I forgot that I was closing CommandPalette at first. Now I changed to close it when action is not toggleCommandPalette.

[zadjii-msft]

Sorry for misleading you on the ctrl+tab thing - let's remove that special case handling, but otherwise this feels good. Thanks!

[zadjii-msft]

Thanks!

[ghost]

@carlos-zamora I made a change. Please review!

[carlos-zamora]

Thanks!

[ghost]

Hello @carlos-zamora!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[DHowett]

@carlos-zamora when you automerge, please try to make sure you edit the PR body to explain what actually happened. It looks like the words in the body of this PR are way, way different from the final implementation! Future readers who only have the git blame will not have the context y’all had when reviewing it.

[DHowett]

Hi @Hegunumo! Sorry about this. I had to revert this pull request because it caused a build break in main.

Reversion: 61ccf44
Reopening #6679

If you make a fix on top of commit 1965fb5, we can merge the two back together.