Skip to content

Conversation

@katysaintin
Copy link
Contributor

backward compatibility for layout management see comment in PR #3250

  • layout_default defines an absolute path to memento
  • layout_dir defines an absolute folder for layout
  • layout argument at launch is take in account as absolute path
  • if layout is not defined , layout_default preferences is checked and manage as absolute path
  • SaveLayout action always save in user location and also in layout_dir if it's defined

boolean statusBarVisible = PhoebusApplication.INSTANCE.isStatusbarVisible();
MementoHelper.saveState(stagesToSave, memento_file, null, null,menuVisible, toolbarVisible, statusBarVisible);
if(memento_custom != null){
MementoHelper.saveState(stagesToSave, memento_custom, null, null,menuVisible, toolbarVisible, statusBarVisible);
Copy link
Collaborator

@abrahamwolk abrahamwolk Feb 27, 2025

Choose a reason for hiding this comment

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

With this change, the layout is saved to both Locations.user() and layout_dir (if it has been set). Would it be possible to only save to Locations.user() by default (this is the old behavior), and to save to layout_dir only when the new functionality to do so has been enabled?

Probably, one wants to save only to one of the locations: layouts are loaded both from Locations.user() [1] and from layout_dir, and if one saves a layout to both, then the layout will appear twice in the Layouts menu.

[1] https://github.com/ControlSystemStudio/phoebus/pull/3300/files#diff-3f6a42b042b32957310098c8d07db1359206386568259c9697e95dfb5f7dd302L791-L797

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello, @abrahamwolk , ok I will see that. How the end users get the memento in a specific layout_dir before ?
Our End users , are not familiar with Phoebus and they even don't know where the default memento are stored.

Did you copy the file manually ?

What can I do, is adding a flag in preferences, by default, keep the old behavior , and if enable, save the layout in the custom layout_dir .
Do you think it's OK with this method ?

Thank you for your help.
Katy

Copy link
Collaborator

Choose a reason for hiding this comment

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

A use-case may be the following:

  • Locations.user() points to a writable user-specific directory, typically located in the user's home directory.
  • layout_dir points to a global read-only directory that contains shared layouts.

Typically, users may modify their own user-specific layouts located in their home directory. Some specific layouts are shared between users, and they may be maintained and modified, e.g., in a shared repository. These shared layouts are then deployed to layout_dir (which is read-only, since the layouts in this directory are shared between several users).

What can I do, is adding a flag in preferences, by default, keep the old behavior , and if enable, save the layout in the custom layout_dir .
Do you think it's OK with this method ?

Yes, I think it's OK!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK I see. Thank you for your answer, I make the modification ASAP.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @abrahamwolk ,

Firstable, thank you for the "use-case" , it is interesting to use layout in this way for the operators control room.
According to the given use case, here are the modifications I propose :

  • Layout files will be saved in layout_dir only if preference save_in_layout_dir is true and if Layout_dir exists and is writable
  • By default save_in_layout_dir=false
  • And all other case layout files are saved in user location.

I hope , it is OK , and it's work for your users .
Thank you for your review .

Katy

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, this is OK. Thanks for fixing it!

@katysaintin katysaintin self-assigned this Feb 27, 2025
@katysaintin
Copy link
Contributor Author

Thank you @abrahamwolk . I think it's OK if I merge so.

Have a nice week end.
Katy

@katysaintin katysaintin merged commit 5508dfc into ControlSystemStudio:master Feb 28, 2025
1 check passed
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