Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Binding enter to copy breaks Command Palette #9044

Closed
LuanVSO opened this issue Feb 5, 2021 · 4 comments
Closed

Binding enter to copy breaks Command Palette #9044

LuanVSO opened this issue Feb 5, 2021 · 4 comments
Assignees
@Don-Vito
Labels
Area-CmdPal Command Palette issues and features Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v1.7

Comments

@LuanVSO
Copy link
Contributor

LuanVSO commented Feb 5, 2021 •
edited
Loading

Environment

Windows build number: 10.0.19042.789
Windows Terminal version (if applicable):1.6.10272.0
Any other software?

Steps to reproduce

  1. put this in actions:
{"command": {"action": "copy","singleLine": false},"keys": "enter"}
  1. try to use cmdpallet

Expected behavior

when enter is pressed in cmdpall the highlighted action is executed

Actual behavior

nothing is executed

Observations

regression from 1.5
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Feb 5, 2021
@zadjii-msft zadjii-msft changed the title binding enter to copy breaks cmdpall Binding enter to copy breaks Command Palette Feb 5, 2021
@zadjii-msft zadjii-msft added Area-CmdPal Command Palette issues and features Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. zPreview-Service-Queued-1.13 A floating label that tracks the current Preview version for servicing purposes. labels Feb 5, 2021
@Don-Vito
Copy link
Contributor

Don-Vito commented Feb 5, 2021 •
edited
Loading

@LuanVSO - this is absolutely a regression.

@zadjii-msft - I think I found why key -binding are not working in settings UI

For some reason the KeyDownHandler of terminal page is bound to PreviewKeyDown of CommandPaletter rather than KeyDown of the terminal page. Fixing this, fixed the SUI binding as well.

Please assign to me 😊

@zadjii-msft
Copy link
Member

@Don-Vito you're the man. I'll throw you on the Assigned-to line, since I had a feeling you had a beat on this one 😄

@zadjii-msft zadjii-msft added this to the Terminal v1.7 milestone Feb 5, 2021
@Don-Vito Don-Vito mentioned this issue Feb 5, 2021
Merged
6 tasks
@ghost ghost added the In-PR This issue has a related PR label Feb 5, 2021
@Don-Vito
Copy link
Contributor

Don-Vito commented Feb 5, 2021 •
edited
Loading

@zadjii-msft - the scope of the issue is wider: copy and paste interact with terminal rather than the search box. I guess it is a good candidate for servicing.

@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Feb 10, 2021
DHowett pushed a commit that referenced this issue Feb 10, 2021
@Don-Vito @DHowett
Fix CommandPalette to prefer inner interactions over bindings (#9056)
3ab1d72 
* 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)
@ghost
Copy link

ghost commented Feb 11, 2021

🎉This issue was addressed in #9056, which has now been successfully released as Windows Terminal Preview v1.6.10412.0.:tada:

Handy links:

@DHowett DHowett removed the zPreview-Service-Queued-1.13 A floating label that tracks the current Preview version for servicing purposes. label Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Don-Vito Don-Vito

Labels
Area-CmdPal Command Palette issues and features Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v1.7
Development

No branches or pull requests
4 participants
@DHowett @Don-Vito @zadjii-msft @LuanVSO