Have "Close All" and friends honor "Cancel" in save prompt dialogs #4977
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When invoking "Close All" on a tab, some tabs may pop up confirmation dialogs asking whether their work should be saved or not. Pressing the "Cancel" button in any dialog, as opposed to "Save" or "Discard", should stop showing further dialogs, and stop the overall operation. This was not previously done. (In some cases, a bunch of dialogs would be shown on top of each other simultaneously, creating visual artifacts.) This change was made to the Close All and Close Other actions, as well as for the newly introduced "Close whole documents list at once" button in the document list dropdown (PR apache#4792). In a previous discussion, it was suggested to make the whole operation atomic (i.e. don't close any tabs at all if the user presses Cancel after several tabs have already been iterated), but this is not possible given the existing APIs. (We don't know if Cancel was pressed until TopComponent.close() was called.) Moreover, the behavior implemented in this PR is more standard in any case; it matches the behavior in e.g. Photoshop and VSCode. It was also suggested to implement similar behavior in the save prompt that shows up when exiting the IDE, but this is a rather different operation that does not actually close any tabs (they remain "open" and persisted for the next time the IDE starts). I'm not quite sure how to improve it; it might require more discussion. See the "Savable" API and the o.n.core.ExitDialog class. I would also have liked to focus each TopComponent in turn when prompts to save come up, but this is a bit more work due to the various levels of abstractions involved. More work on this can be done later. See MultiViewFactory.resolveCloseOperation for a relevant starting point.
eirikbakke
added
Platform
mbien
approved these changes
Nov 16, 2022
There was a problem hiding this comment.
code looks good to me.
Tested it and it works as advertised. Cancels the action etc.
Don't worry about the atomic action suggestion, it was just an idea to reuse the multi-file-save dialog for it and ask questions first before running the close op. I do see that this would be more difficult to implement and probably not worth the trouble since this solution works really well already. (in most cases everything will close without asking questions anyway since docs are likely saved most of the time)
|
Thanks for testing, @mbien! Merged now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When invoking "Close All" on a tab, some tabs may pop up confirmation dialogs asking whether their work should be saved or not. Pressing the "Cancel" button in any dialog, as opposed to "Save" or "Discard", should stop showing further dialogs, and stop the overall operation. This was not previously done. (In some cases, a bunch of dialogs would be shown on top of each other simultaneously, creating visual artifacts.)
This change was made to the Close All and Close Other actions, as well as for the newly introduced "Close whole documents list at once" button in the document list dropdown (PR #4792).
In a previous discussion, it was suggested to make the whole operation atomic (i.e. don't close any tabs at all if the user presses Cancel after several tabs have already been iterated), but this is not possible given the existing APIs. (We don't know if Cancel was pressed until TopComponent.close() was called.) Moreover, the behavior implemented in this PR is more standard in any case; it matches the behavior in e.g. Photoshop and VSCode.
It was also suggested to implement similar behavior in the save prompt that shows up when exiting the IDE, but this is a rather different operation that does not actually close any tabs (they remain "open" and persisted for the next time the IDE starts). I'm not quite sure how to improve it; it might require more discussion. See the "Savable" API and the o.n.core.ExitDialog class.
I would also have liked to focus each TopComponent in turn when prompts to save come up, but this is a bit more work due to the various levels of abstractions involved. More work on this can be done later. See MultiViewFactory.resolveCloseOperation for a relevant starting point.