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

Toolbar entry for Logbook #3163

Merged
merged 1 commit into from
Oct 11, 2024
Merged

Toolbar entry for Logbook #3163

merged 1 commit into from
Oct 11, 2024

Conversation

georgweiss
Copy link
Collaborator

Some additional changes:

Removed option to customize display name for "Log Entry Table" app. The display name is now "Logbook" set in messages.properties file.

@shroffk
Copy link
Member

shroffk commented Oct 11, 2024

ah this is nice...

@shroffk shroffk merged commit f33eeb3 into master Oct 11, 2024
2 checks passed
@tynanford
Copy link
Contributor

So this preference setting is no longer available? org.phoebus.logbook.olog.ui/log_entry_table_display_name

If so that is too bad, we are using that here. org.phoebus.logbook.olog.ui/log_entry_calendar_display_name might as well be deleted too?

@shroffk
Copy link
Member

shroffk commented Oct 11, 2024

Ok,
Maybe we can add the preference back in then.
What is the name you use @tynanford?

Should we in general use Messages for UI representation ( so that we can switch languages )...and what is the role that preferences should play here?

@georgweiss
Copy link
Collaborator Author

Oh dear, I was under the impression that only ESS users were unhappy with "Log Entry Table" and wanted a customized name like Logbook. In any case, reason I chose to go for a (potentially localized) string in Messages is that the preference did override the default name, which in turn had an impact on the toolbar customization. See also #3014 for additional aspects of localization and potential remedy.

@tynanford
Copy link
Contributor

It's not a huge deal but I feel like we should be careful about removing preference settings since we can't know who uses them. In our case the phoebus OLOG integration is for a very specific purpose so we use Tuning Logbook (notes by operators on tuning the injector). Of course if this preference wasn't around we would have just told them to use the "Log Entry Table" button for these notes and we can always go with Logbook in the future.

I haven't looked into the Messages property files and how they come into play but see that is how the French UI support is implemented. Is that the recommended way to change specific UI text in phoebus? If so that is fine for us

@georgweiss
Copy link
Collaborator Author

Well, changing Messages is actually about changing messages.properties. This is not very flexible.
I do not mind adding back the preference, but then I would need to tweak some more code to also consider localization.

@georgweiss
Copy link
Collaborator Author

I will push a PR addressing the issue mentioned by @tynanford, and and the same time resolve application name localization as dicussed #3014.

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