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.

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

Re-add keybindings to new tab dropdown #3618

Merged
merged 3 commits into from
Nov 21, 2019

Conversation

zadjii-msft
Copy link
Member

Summary of the Pull Request

With #3391, I almost certainly regressed the ability for the new tab dropdown to display the keybindings for each profile. This adds them back.

PR Checklist

Detailed Description of the Pull Request / Additional comments

Now, we can lookup keybindings not only for ShortcutActions, but also ActionAndArgss, so we can look up the binding for an action with a particular set of arguments.

@zadjii-msft zadjii-msft added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. labels Nov 18, 2019
Copy link
Contributor

@DHowett-MSFT DHowett-MSFT left a comment

Choose a reason for hiding this comment

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

Joke about expense aside, thanks for fixing 😄

// If NewTabProfileN didn't have a binding, look for a
// keychord that is bound to the equivalent
// NewTab(ProfileIndex=N) action
auto actionAndArgs = winrt::make_self<winrt::TerminalApp::implementation::ActionAndArgs>();
Copy link
Contributor

Choose a reason for hiding this comment

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

omg the expense

@zadjii-msft zadjii-msft added the Needs-Second It's a PR that needs another sign-off label Nov 18, 2019
@ghost ghost requested review from miniksa and carlos-zamora November 18, 2019 20:42
src/cascadia/TerminalApp/TerminalPage.cpp Outdated Show resolved Hide resolved
Co-Authored-By: Carlos Zamora <carlos.zamora@microsoft.com>
@zadjii-msft
Copy link
Member Author

I can't block my own change, but this needs to be merged nicely with #3629

# Conflicts:
#	src/cascadia/TerminalApp/ActionArgs.h
@zadjii-msft
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@zadjii-msft zadjii-msft merged commit 71debc1 into master Nov 21, 2019
@zadjii-msft zadjii-msft deleted the dev/migrie/b/3603-new-tab-dd-regression branch January 31, 2020 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Needs-Second It's a PR that needs another sign-off Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dropdown menu no longer displays bound keys (after #3391)
3 participants