Skip to content

Add actions missing in schema, descriptions for toggleRetroEffect#6806

Merged
3 commits merged intomasterfrom
dev/migrie/f/command-palette-merge-nits
Jul 7, 2020
Merged

Add actions missing in schema, descriptions for toggleRetroEffect#6806
3 commits merged intomasterfrom
dev/migrie/f/command-palette-merge-nits

Conversation

@zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Jul 6, 2020 •
edited by DHowett
Loading

Summary of the Pull Request

In the wake of #6635, a couple things got missed in merges:

  • toggleRetroEffect didn't get into the schema, nor did renameTab or
    commandPalette.
  • toggleRetroEffect also didn't get a name

Furthermore, I thought it might be a good idea to start sticking
commands into bindings even without keys. So I tried doing that for
opentabColorPicker and toggleRetroEffect, and found immediately that
the labels for the key chord still appear even when the text is empty.
So I added some XAML magic to hide those when the text is empty.

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Removed all my manual actions, ran the Terminal:
image

@zadjii-msft
These are very minor missed bits of merges
7dbe421 
  * Add `commandPalette` et. al. to the schema
  * Add a description for the toggle retro effect command
@zadjii-msft
Add these commands to the defaults, and hide the keychord label when …
41a8a8f 
…a command doesn't have keys bound
@ghost ghost added Area-Schema Things that have to do with the json schema. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. labels Jul 6, 2020
DHowett
DHowett approved these changes Jul 7, 2020
View reviewed changes
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value converter! I love it!

@DHowett
Copy link
Member

DHowett commented Jul 7, 2020

/cc @leonMSFT for some more xaml magic

@DHowett DHowett added the Needs-Second It's a PR that needs another sign-off label Jul 7, 2020
@ghost ghost requested review from carlos-zamora, leonMSFT and miniksa July 7, 2020 20:54
leonMSFT
leonMSFT approved these changes Jul 7, 2020
View reviewed changes
Copy link
Contributor

@leonMSFT leonMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wtf
wtf that's so neat

@DHowett DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jul 7, 2020
@ghost
Copy link

ghost commented Jul 7, 2020

Hello @DHowett!

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.

@ghost ghost merged commit 5bc31a1 into master Jul 7, 2020
@ghost ghost deleted the dev/migrie/f/command-palette-merge-nits branch July 7, 2020 21:46
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DHowett DHowett DHowett approved these changes

@miniksa miniksa Awaiting requested review from miniksa

@carlos-zamora carlos-zamora Awaiting requested review from carlos-zamora

+1 more reviewer

@leonMSFT leonMSFT leonMSFT approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area-Schema Things that have to do with the json schema. AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Second It's a PR that needs another sign-off Product-Terminal The new Windows Terminal.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add toggleRetroEffect to schema

3 participants

@zadjii-msft @DHowett @leonMSFT