Skip to content

Conversation

@mpela81
Copy link
Contributor

@mpela81 mpela81 commented Apr 16, 2021

Summary of the Pull Request

Add the "Close other tabs"/"Close tabs to the right" menu items straight to the tab context menu to work around #8238.
We can't add them into a dedicated sub-menu until the upstream crash is fixed.

References

#8238

PR Checklist

  • Closes Crash opening/closing close... submenu multiple times #8238
  • CLA signed. If not, go over here and sign the CLA
  • Tests added/passed
  • Documentation updated
  • Schema updated.
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan.

Detailed Description of the Pull Request / Additional comments

Moved the creation of the close menu items to a single function. Once the originating crash is fixed, the sub-menu can be restored by just replacing a few lines of code.

Validation Steps Performed

immagine

Do not add a submenu until microsoft#8238 crash is fixed.
@ghost ghost added Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news. labels Apr 16, 2021
@mpela81 mpela81 marked this pull request as ready for review April 17, 2021 16:53
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.

You know, as much as I love dangling the original bug as a carrot to get the underlying bug fixed, it's not a good experience for our users. Let's just ship this for now, and hopefully we'll get the nested flyouts sometime soon.

Copy link
Contributor

@PankajBhojwani PankajBhojwani left a comment

Choose a reason for hiding this comment

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

Makes sense to me, thank you!

@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Apr 21, 2021
@ghost
Copy link

ghost commented Apr 21, 2021

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.

@ghost ghost merged commit 2065fa7 into microsoft:main Apr 21, 2021
@mpela81 mpela81 deleted the feat/close_items branch April 21, 2021 10:54
DHowett pushed a commit that referenced this pull request May 14, 2021
## Summary of the Pull Request
Add the "Close other tabs"/"Close tabs to the right" menu items straight to the tab context menu to work around #8238.
We can't add them into a dedicated sub-menu until the upstream crash is fixed.

## References
#8238

## PR Checklist
* [X] Closes #8238
* [X] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan.

## Detailed Description of the Pull Request / Additional comments
Moved the creation of the close menu items to a single function. Once the originating crash is fixed, the sub-menu can be restored by just replacing a few lines of code.

## Validation Steps Performed
![immagine](https://user-images.githubusercontent.com/1140981/115059601-0dbc2480-9ee7-11eb-9889-d9ef8e6e7613.png)

(cherry picked from commit 2065fa7)
DHowett pushed a commit that referenced this pull request May 14, 2021
Add the "Close other tabs"/"Close tabs to the right" menu items straight to the tab context menu to work around #8238.
We can't add them into a dedicated sub-menu until the upstream crash is fixed.

* [X] Closes #8238
* [X] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan.

Moved the creation of the close menu items to a single function. Once the originating crash is fixed, the sub-menu can be restored by just replacing a few lines of code.

![immagine](https://user-images.githubusercontent.com/1140981/115059601-0dbc2480-9ee7-11eb-9889-d9ef8e6e7613.png)

(cherry picked from commit 2065fa7)
(cherry picked from commit c4e02e7)
DHowett pushed a commit that referenced this pull request May 14, 2021
Add the "Close other tabs"/"Close tabs to the right" menu items straight to the tab context menu to work around #8238.
We can't add them into a dedicated sub-menu until the upstream crash is fixed.

* [X] Closes #8238
* [X] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan.

Moved the creation of the close menu items to a single function. Once the originating crash is fixed, the sub-menu can be restored by just replacing a few lines of code.

![immagine](https://user-images.githubusercontent.com/1140981/115059601-0dbc2480-9ee7-11eb-9889-d9ef8e6e7613.png)

(cherry picked from commit 2065fa7)
(cherry picked from commit c4e02e7)
@ghost
Copy link

ghost commented May 25, 2021

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

Handy links:

@ghost
Copy link

ghost commented May 25, 2021

🎉Windows Terminal Preview v1.9.1445.0 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

Labels

Area-UserInterface Issues pertaining to the user interface of the Console or Terminal 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-1 A description (P1) Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash opening/closing close... submenu multiple times

3 participants