Skip to content
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

CSSTUDIO-1761 (Part 1/2) Add functionality to the menu of the main window for adding a layout to the current layout. #2550

Merged
merged 8 commits into from
Feb 27, 2023

Conversation

abrahamwolk
Copy link
Collaborator

@abrahamwolk abrahamwolk commented Feb 15, 2023

CSSTUDIO-1761 asks for implementation of:

  1. The functionality to save the layout of individual windows.
  2. The functionality to add windows that have been saved in this way to the current layout.

This merge-request implements point 2. Point 1. is implemented in merge-request #2551.

This merge-request adds a sub-menu called "Add Layout" under the menu "Window" of the main window of the Phoebus application. In contrast to the sub-menu "Load Layout", "Add Layout" doesn't replace the current layout with the loaded layout, instead it adds the loaded layout in by opening one or more new windows, leaving the currently opened layout intact.

The functionality implemented in this merge-request is more general than what is described under point 2.: not only individual windows can be added to the current layout, but any layout (with any number of windows) that has been saved.

The main part of the implementation of "Add Layout" is carried out in the function addLayoutToCurrentLayout() in the class PhoebusApplication. The implementation leverages the existing functionality of "restoring" stages from objects of type MementoTree by calling the function MementoHelper.restoreStage().

MementoHelper.restoreStage() (re-)creates the individual applications of a stored layout by invoking the corresponding instances of AppDescriptor.create() for the applications in question. Many (but not all) applications implement functionality in AppDescriptor.create() that locates an already existing copy of the application in question, which, if such an instance is found, does not create a new instance of the application, but instead puts the focus on the existing instance and possibly refreshes its content.

The functionality that AppDescriptor.create() often implements of selecting and possibly refreshing an already existing instance instead of creating a new instance leads to three complications for the implementation of the "Add Layout" sub-menu. If a new instance of an application is not created, but instead an existing instance is given focus, then:

  1. the pane in the loaded layout that would contain the application is left empty. If all panes of a window in the loaded layout are empty, then the window in question should not be loaded (or, at the least, it should quickly be automatically closed again);
  2. the focus of the tab containing the existing instance (that was focused by AppDescriptor.create()) will be opened, possibly changing which tab is opened in a window before "Add Layout" was invoked. This should not be the case, since the windows that were already open should be unchanged after invoking "Add Layout";
  3. the existing instance may be refreshed by AppDescriptor.create(). This should not happen, since the windows that were already open should be unchanged after invoking "Add Layout".

Points 1. and 2. have been addressed, but I have not found a way to address point 3.

Point 1. is addressed by iterating through the newly created instances of Stage, checking whether they contain any tabs. If an instance doesn't contain any tabs, then no application was restored to the instance in question, and the stage in question is closed. The main difficulty in implementing this check is that it can only be performed once every created instance of Pane has an associated instance of Scene. To this end, the function DockStage.deferUntilAllPanesOfStageHaveScenes() was implemented. DockStage.deferUntilAllPanesOfStageHaveScenes() is implemented using DockPane.deferUntilInScene(), whose implementation was improved (also fixing #2544):

  • It is now implemented using a ChangeListener instead of by trying to wait.
  • The relative ordering in time of deferred function calls is preserved: if f1() is deferred before f2() is deferred, then f1() will be invoked before f2() is invoked. This is necessary in order to guarantee that the check for whether any tabs have been restored is run after the code restoring instances is run.

Point 2. is addressed by iterating through the existing panes of the application before creating new stages, saving the selected item in each pane. After the new stages have been restored, the selected items of each original pane are restored. Finally, the newly created stages are given focus.

Point 3. is, as stated above, not addressed, and any suggestions on how to address point 3. are very welcome.

@abrahamwolk abrahamwolk requested a review from kasemir February 15, 2023 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants