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

Add an accelerator key for the shell extension #13080

Merged
2 commits merged into from
Jun 10, 2022
Merged

Add an accelerator key for the shell extension #13080

2 commits merged into from
Jun 10, 2022

Conversation

ianjoneill
Copy link
Contributor

@ianjoneill ianjoneill commented May 11, 2022 •
edited by DHowett
Loading

Adds an accelerator key for the shell extension: T for stable, P for preview and D for dev.

Validation

Ran a dev build and saw the keyboard accelerator assigned.

Closes #13061

@ghost ghost added Area-ShellExtension For issues related to the explorer right-click context menu Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels May 11, 2022
@github-actions

This comment was marked as resolved.

Sign in to view
@ianjoneill
Copy link
Contributor Author

How do I appease the spelling bot? I don't think adding iew as an allowed word is the right approach. Ideally there would be some way of stating this is OK for this one file, but I don't see how you do that.

@zadjii-msft zadjii-msft added the Needs-Discussion Something that requires a team discussion before we can proceed label May 16, 2022
@zadjii-msft
Copy link
Member

Hokay so some notes from discussion:

  • We like the idea
  • I think we liked (W, P, and D) as characters anyways - there's always gonna be conflicts with these letters, so might as well go with sensible options.
  • Localization for this might be Hard. We're gonna look into it on our end and see what the right way to markup these strings is. We need to make sure the translators know that the & identifies the leading character as an accelerator, and for non-latin-character-based languages, the localizations have a trailing latin character for loc purposes.

So we're gonna hold this for 1.15, while we sort out that loc issue. Thanks for your patience!

@ianjoneill
Copy link
Contributor Author

So to confirm, you'd like D for dev, P for preview and W for stable? i.e. for English:

Open in Terminal (&Dev)
Open in Terminal &Preview
Open in Terminal (&w)

@zadjii-msft zadjii-msft self-assigned this May 23, 2022
@zadjii-msft
Copy link
Member

Oh my god I'm so sorry, I clearly didn't even read that comment before I posted it. I meant T, not W. 🤦

Open in Terminal (&Dev)
Open in Terminal &Preview
Open in &Terminal

The action item is still on me to follow up with the File Explorer team (or someone...?) to figure out the right way to express these for future localization. Sorry for the delays, releasing 1.14 has been a persistent fire drill

@ianjoneill
Copy link
Contributor Author

Yeah I figured it was probably a typo - hence why I questioned it rather than blindly applying it 😛

DHowett
DHowett approved these changes Jun 10, 2022
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.

I'm cool with this -- we can "progressively render" the cross-language accelerator story. What do you think, @zadjii-msft?

@DHowett
Copy link
Member

DHowett commented Jun 10, 2022

Thanks @ianjoneill :)

@DHowett
Copy link
Member

DHowett commented Jun 10, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

zadjii-msft
zadjii-msft approved these changes Jun 10, 2022
View reviewed changes
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

I'm cool with that.

@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jun 10, 2022
@ghost
Copy link

ghost commented Jun 10, 2022

Hello @zadjii-msft!

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.

@ianjoneill
Copy link
Contributor Author

The CI officially hates me - if I merge main into this branch will it still auto-merge, or is it better for you to re-trigger the build yourself?

@zadjii-msft
Copy link
Member

Just triggered a rerun of the feature tests, they've been flaky all day, sorry 😕

@ianjoneill
Copy link
Contributor Author

Thanks! I don't know why, but I seem to be particularly unlucky with build flakes.

@ghost ghost merged commit 75e4624 into microsoft:main Jun 10, 2022
@zadjii-msft zadjii-msft removed the Needs-Discussion Something that requires a team discussion before we can proceed label Jun 13, 2022
DHowett pushed a commit that referenced this pull request Jun 30, 2022
@ianjoneill @DHowett
Add an accelerator key for the shell extension (#13080)
102f612 
Adds an accelerator key for the shell extension: `T` for stable, `P` for preview and `D` for dev.

# Validation
Ran a dev build and saw the keyboard accelerator assigned.

Closes #13061

(cherry picked from commit 75e4624)
Service-Card-Id: 83524177
Service-Version: 1.14
@ghost
Copy link

ghost commented Jul 6, 2022

🎉Windows Terminal v1.14.186 has been released which incorporates this pull request.:tada:

Handy links:

@ghost ghost mentioned this pull request Jul 6, 2022
Closed
@ghost
Copy link

ghost commented Jul 6, 2022

🎉Windows Terminal Preview v1.15.186 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zadjii-msft zadjii-msft zadjii-msft approved these changes

@DHowett DHowett DHowett approved these changes

Assignees

@zadjii-msft zadjii-msft

Labels
Area-ShellExtension For issues related to the explorer right-click context menu AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"Open In Terminal" doesn't have a letter selector or hot key
3 participants
@ianjoneill @zadjii-msft @DHowett