Double middle click on taskbar preview closes application #7871
Conversation
When a close command is invoked (middle click on taskbar preview or 'X' button), new flag is set. When the user wants to close again (this time only via the taskbar preview, as the 'X' button is disabled), the application is closed. If the user cancels the dialog, the flag is reset to prevent accidental closing on a subsequent close command.
ghost
added
Area-User Interface
|
Let me know if my approach isn't state of the art. |
|
This is clever, and I like the feature. Nothing bothers me more than when Vim doesn't exit because it wanted to ask if I was sure 😄. Thanks! You didn't cause the UT failure -- they can just be a bit flaky. We'll get on reviewing this tomorrow/tuesday 😄 |
|
If this is ready for the team to look at, make sure you unmark it as Draft! We usually don't look at drafts too quickly. (EDIT: I deleted the smiley because holy heck did I use a lot in the past 2 minutes.) |
|
Failure: I'm glad. |
There was a problem hiding this comment.
This is great. Couple thoughts:
- I think we should flip the sense of the boolean (
_displayingCloseDialoginstead of_isFirstClose) - We need to eventually refactor (you definitely don't need to do it for this PR!) dialog display to be "awaitable", so we don't need to add an event handler to the close button.
That would make dialog display more like this:
_displayingCloseDialog = true;
co_await _ShowCloseDialog();
_displayingCloseDialog = false;
|
I had a shot at refactoring the dialog 😄. See commit. |
zadjii-msft
added
the
AutoMerge
|
Hello @zadjii-msft! 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 (
|
ghost
merged commit
|
I'm happy I could help. About refactoring the dialog: Is it okay that I create a PR directly (and mention this PR) or do you prefer that I first create an issue for it? |
|
Meh, if the code's already done, then just make the second PR and reference this one in it. No need to make an issue to say "Do the thing I've done in {PR}" 😆 |
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request A second close command (middle click on taskbar preview) overrides the warning dialog and closes the application. <!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> ## References <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist * [x] Closes #7451 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [ ] Tests added/passed * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [ ] 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. Issue number where discussion took place: #xxx <!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments When a close command is invoked (middle click on taskbar preview or 'X' button), a new flag is set. When the user wants to close again (this time only via the taskbar preview, as the 'X' button is disabled), the application is closed. If the user cancels the dialog, the flag is reset to prevent accidental closing on a subsequent close command. <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed I am developing with a [Windows 10 virtual machine](https://developer.microsoft.com/en-us/windows/downloads/virtual-machines/) provided by Microsoft. I tested manually. I considered the 'X' button, middle click on taskbar preview, and Alt+F4. Only a middle click on the taskbar preview does override the dialog. (cherry picked from commit d1e58bd)
|
🎉 Handy links: |
ghost
mentioned this pull request
|
🎉 Handy links: |
The CloseWarningDialog is now "awaitable"/async, as suggested in PR #7871. As opening the dialog is async, the flag can be reset in the same method. This way the flag operations occur in the same method. The event handlers of the buttons became obsolete and are removed. ## Validation Steps Performed Tested manually.
The CloseWarningDialog is now "awaitable"/async, as suggested in PR microsoft#7871. As opening the dialog is async, the flag can be reset in the same method. This way the flag operations occur in the same method. The event handlers of the buttons became obsolete and are removed. ## Validation Steps Performed Tested manually.
Summary of the Pull Request
A second close command (middle click on taskbar preview) overrides the warning dialog and closes the application.
References
PR Checklist
Detailed Description of the Pull Request / Additional comments
When a close command is invoked (middle click on taskbar preview or 'X' button), a new flag is set. When the user wants to close again (this time only via the taskbar preview, as the 'X' button is disabled), the application is closed. If the user cancels the dialog, the flag is reset to prevent accidental closing on a subsequent close command.
Validation Steps Performed
I am developing with a Windows 10 virtual machine provided by Microsoft. I tested manually. I considered the 'X' button, middle click on taskbar preview, and Alt+F4. Only a middle click on the taskbar preview does override the dialog.