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-2648 "Add Layout" toolbar button and new "Add Layout" icon #3227

Merged
merged 3 commits into from
Jan 9, 2025

Conversation

abrahamwolk
Copy link
Collaborator

@abrahamwolk abrahamwolk commented Jan 9, 2025

This pull request:

  1. Adds a new toolbar button "Add Layout":

screenshot

  1. Adds a new icon for adding layouts. The new icon is used both for the new toolbar button as well as for the sub-menu "Window" -> "Add Layout" in the menu bar.

The new toolbar button is disabled by default and must be added explicitly by setting org.phoebus.ui/toolbar_entries=Add Layouts.

Copy link
Collaborator

@georgweiss georgweiss left a comment

Choose a reason for hiding this comment

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

Variable names like add_layout do no conform to Java naming convention.

Why is the button in the toolbar is a dropdown?

@abrahamwolk
Copy link
Collaborator Author

Variable names like add_layout do no conform to Java naming convention.

It conforms to the naming of the other instances of MenuButton in the class. I think it would be less readable to use camel case when the other instances of MenuButton don't use it.

Why is the button in the toolbar is a dropdown?

It's a dropdown that lets you choose the layout to be added. This is analogous to the already existing "Load Layouts" dropdown.

@georgweiss
Copy link
Collaborator

It conforms to the naming of the other instances of MenuButton in the class. I think it would be less readable to use camel case when the other instances of MenuButton don't use it.

I beg to disagree.

It's a dropdown that lets you choose the layout to be added. This is analogous to the already existing "Load Layouts" dropdown.

OK, I see. I thought this was about creating a new layout...

@shroffk shroffk merged commit f12b02e into master Jan 9, 2025
2 checks passed
@abrahamwolk abrahamwolk deleted the CSSTUDIO-2648 branch January 10, 2025 07:26
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.

4 participants