Skip to content

commandPalette keybinding should close the palette when it's open #6679

Description

This is being moved from discussion in #6635.

We wanted to enable the commandPalette keybinding to also work within the palette itself.

carlos-zamora

I know I suggested this, but it unveiled a huge problem. When paste is bound to ctrl+v, we paste to the underlying terminal twice. A similarly weird problem occurs with copy.

Maybe we should only detect the commandPalette action and use that as an additional way to dismiss it.

DHowett

Don't we just need to mark it Handled here? lol

zadjii-msft

This is shockingly bad actually.

  • If we just mark it as Handled(true), then the following KeyUp will still land in the TermControl, resulting in ^VPasted Text being written to the Terminal.
  • If we change the keybinding handler to on KeyUp, then it's likely that the first keyup it'll see is the keybinding for toggle the palette, which will immediately close it.

I'm going to move this to a follow-up issue, to get at the least the toggle the command palette action back, and maybe the full keybindings

[3:29 PM] Mike Griese
I'll file a follow-up - maybe we could do something where the action is selected on the keydown, but then only dispatched on the keyup?
​> [3:30 PM] Mike Griese
so we'll only try a keybinding on a keyup after a non-modifier keydown

Activity

  1. ghost added
    Needs-TriageIt's a new issue that the core contributor team needs to triage at the next triage meeting
    on Jun 25, 2020
  2. ghost added
    Needs-Tag-FixDoesn't match tag requirements
    on Jun 25, 2020
  3. added
    Area-UserInterfaceIssues pertaining to the user interface of the Console or Terminal
    Issue-TaskIt's a feature request, but it doesn't really need a major design.
    on Jun 25, 2020
  4. ghost removed
    Needs-Tag-FixDoesn't match tag requirements
    on Jun 25, 2020
  5. removed
    Needs-TriageIt's a new issue that the core contributor team needs to triage at the next triage meeting
    on Jun 25, 2020
  6. ghost added
    In-PRThis issue has a related PR
    on Dec 3, 2020
  7. ghost closed this as completedin #8044on Dec 14, 2020
  8. ghost removed
    In-PRThis issue has a related PR
    on Dec 14, 2020
  9. ghost added
    Resolution-Fix-CommittedFix is checked in, but it might be 3-4 weeks until a release.
    on Dec 14, 2020
  10. ghost added a commit that references this issue on Dec 14, 2020
    1965fb5
  11. DHowett commented on Dec 14, 2020

    @DHowett
    Member

    Reopened -- reverted PR

  12. ghost added
    In-PRThis issue has a related PR
    on Dec 14, 2020
  13. ghost removed
    Resolution-Fix-CommittedFix is checked in, but it might be 3-4 weeks until a release.
    on Dec 14, 2020
  14. ghost closed this as completedin #8586on Dec 18, 2020
  15. ghost removed
    In-PRThis issue has a related PR
    on Dec 18, 2020
  16. ghost added
    Resolution-Fix-CommittedFix is checked in, but it might be 3-4 weeks until a release.
    on Dec 18, 2020
  17. ghost added a commit that references this issue on Dec 18, 2020
    01a0490
  18. added 2 commits that reference this issue on Jan 28, 2021
    4c82a7c
    65db5b1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area-CmdPalCommand Palette issues and featuresArea-UserInterfaceIssues pertaining to the user interface of the Console or TerminalIssue-TaskIt's a feature request, but it doesn't really need a major design.Priority-2A description (P2)Product-TerminalThe new Windows Terminal.Resolution-Fix-CommittedFix is checked in, but it might be 3-4 weeks until a release.

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions