-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
Add find item to tab menu #13055
Add find item to tab menu #13055
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Thanks!
Hello @carlos-zamora! Because this pull request has the 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 (
|
@@ -1273,6 +1273,23 @@ namespace winrt::TerminalApp::implementation | |||
exportTabMenuItem.Icon(exportTabSymbol); | |||
} | |||
|
|||
Controls::MenuFlyoutItem findMenuItem; | |||
{ | |||
// "Split Tab" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oop - this is find!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah sorry about that, obviously happened during copy-paste 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution! :)
## Summary of the Pull Request Add `Find` to tab context menu as describe in issue #5633. ## PR Checklist * [x] Closes #5633 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA ## Detailed Description of the Pull Request / Additional comments Just wanted to solve `Easy Starter` issue, so any corrections/suggestions welcome. There's a couple of points I'm not sure of * Placement of item within menu, currently it's at the end before close tab block. * Should it be named longer, something like `Find in Tab` of just `Find` is fine? * The workaround for focus similar to tab rename is a bit annoying, especially because it required adding a method to `TermControl` but without it find window obviously opens without focus which is bad. ## Validation Steps Performed Open menu, press menu item try to find things via opened find dialog. (cherry picked from commit b851b0d) Service-Card-Id: 83524184 Service-Version: 1.14
🎉 Handy links: |
🎉 Handy links: |
Summary of the Pull Request
Add
Find
to tab context menu as describe in issue #5633.PR Checklist
Detailed Description of the Pull Request / Additional comments
Just wanted to solve
Easy Starter
issue, so any corrections/suggestions welcome. There's a couple of points I'm not sure ofFind in Tab
of justFind
is fine?TermControl
but without it find window obviously opens without focus which is bad.Validation Steps Performed
Open menu, press menu item try to find things via opened find dialog.