CSSTUDIO-1987 New "Unsaved Changes" confirmation dialog

[abrahamwolk]

This merge-request implements a new dialog when closing multiple tabs, as for instance when exiting "Phoebus" or running the command "Close All Tabs".

Instead of each tab displaying its own confirmation window (which can be difficult to locate), the changes to be saved or discarded are summarized in one window (there are several variations depending on which command is run):

If an individual tab is closed, the original confirmation dialog is still shown. The text of the dialog, however, has been slightly updated.

Previous to this merge-request, the confirmation dialogs of instances of DockItemWithInput were added as event handlers when a close-request was received. This has been changed: now the code for the confirmation dialogs is explicitly called when commands for closing tabs are called. I believe that this improves the behavior in cases where the user cancels the close-action, since either all prepareToClose() methods will be invoked or none (in the case of cancellation).

Additional screenshots
When some of the instances with changes have been saved:

When all instances with changes have been selected:

When a save operation is in progress:

When a "Save as..." operation was cancelled by the user:

[kasemir]

The end result looks great, and I like how the changes to applications are minimal.

Slightly worried because the shutdown is complicated: Closing of one or all apps is initiated on the UI thread, then we perform the actual writing of files in background threads, eventually reacting to success or failure back on the user thread.
I find it impossible to tell if all is "correct" just from looking at the changes.

Have you tried pushing all the buttons for like an hour in various ways and found no hangup?

[abrahamwolk]

The end result looks great, and I like how the changes to applications are minimal.

Thank you!

Slightly worried because the shutdown is complicated: Closing of one or all apps is initiated on the UI thread, then we perform the actual writing of files in background threads, eventually reacting to success or failure back on the user thread. I find it impossible to tell if all is "correct" just from looking at the changes.

Have you tried pushing all the buttons for like an hour in various ways and found no hangup?

No, I have not, and during development I did encounter unexpected bugs. I can spend some time tomorrow to test the implementation, and also to review it myself again. But bugs can of course remain.

One issue that I can think of is: suppose the the confirmation dialog is displayed, but then some background process causes an application instance that previously (when the confirmation dialog was created) did not have unsaved changes to have unsaved changes. (For instance, previously, incoming data points could trigger the autoscale-functionality of the Data Browser to mark the Data Browser as having unsaved changes.)

In the current implementation, these new unsaved changes will simply be ignored if the user proceeds to close the application. Is this something that needs to be taken into account?

[kasemir]

I can spend some time tomorrow to test ..

Yes, that sounds good, gives me a warm fuzzy.

new unsaved changes will simply be ignored

That's fine. "Close" checks what needs to be saved at that time, saves, done. If new changes come in, they're lost, which is fine because they're after the point in time where we started the closing.

[abrahamwolk]

Suppose there are two different instances of the Data Browser, and they both are saved to the same file. Currently, both instances will be saved to the same file, the second one overwriting the first one. Should the "Unsaved Changes" dialog attempt to detect this problem?

[abrahamwolk]

Suppose there are two different instances of the Data Browser, and they both are saved to the same file. Currently, both instances will be saved to the same file, the second one overwriting the first one. Should the "Unsaved Changes" dialog attempt to detect this problem?

One alternative could perhaps be to modify the save procedure such that it is not possible to save to the same file as another opened Data Browser.

It is problematic when there are two running instances of the Data Browser associated with the same file, also without the "Unsaved Changes" dialog: for instance, if one such instance runs in the Main Window and the other such instance runs in a Secondary Window without any other application instances in the Secondary Window, then when I close Phoebus and open it again, the second window will be restored but without any content. I am also unable to close the window, but must instead remove or edit the file ~/.phoebus/memento. Therefore, perhaps the program state where two instances share the same input file should be avoided in the first place.

[kasemir]

two different instances of the Data Browser, and they both are saved to the same file.

That would almost require bad intent. Open x.plt -> it opens. Open x.plt again -> The existing instance pops to the front.

User creates new plot, saves as y.plt -> OK. User creates another plot, tries to also save as y.plt -> Warning that the file already exists.

If a user insists on saving two plots to the same file, it's their own fault.

[abrahamwolk]

That would almost require bad intent. Open x.plt -> it opens. Open x.plt again -> The existing instance pops to the front.

User creates new plot, saves as y.plt -> OK. User creates another plot, tries to also save as y.plt -> Warning that the file already exists.

If a user insists on saving two plots to the same file, it's their own fault.

The user is given a warning that the file exists, but not that it is already opened. Perhaps there are many monitors with many tabs, and the user is simply unaware that the file is still open somewhere.

I think that the save-functionality should be amended to check for whether the file is already opened or not.

[abrahamwolk]

I discovered a bug in #2759 where sometimes the wrong tab would be selected. I have added the fix to this pull-request, since it is part of the code that is part of the event handler that I have combined in this PR. The fix consists of always running the tab-selection code on the JavaFX-thread using Platform.runLater().

[abrahamwolk]

This PR relies on the assumption that instances of DockItemWithInput are the only tabs that may have changes that can be saved. Is this assumption correct?

[kasemir]

Yes, DockItemWithInput are the only tabs that may have changes that can be saved. Mind you, any DockItem can store/restore state from a memento, but the clean/dirty state that goes with save/saveAs is limited to DockItemWithInput.

[abrahamwolk]

Thanks! I believe that this PR is ready to be merged then.