Add property to configure the 'not_enabled' css styling for widgets

[rjwills28]

I have implemented the changes required to allow the 'not_enabled' css style to be configured by facilities using a Phoebus property. This would allow users to override the default 40% opacity if this doesn't suit their needs.

This PR addresses the feature request in issue #3590.

Note: if the property is not set then the default styling will be used so current users will see no changes.

Checklist

• Testing:

  • The feature has automated tests
  • Tests were run
  • If not, explain how you tested your changes

• Documentation:

  • The feature is documented - comment in the properties file, which will get automatically picked up when generating preferences documentation.
  • The documentation is up to date
  • Release notes:
    • Added an entry if the change is breaking or significant
    • Added an entry when adding a new feature

[kasemir]

The original code always called Styles.update(..., !enabled), so no matter if enabled or not, the style was updated.
Now you have if (children != null && !enabled) ... setStyle which looks like it will apply that style when disabled, but how is the widget ever getting re-enabled? Note that enablement can change while the display is running, so it's not sufficient to disable, also need to be re-enable when the time comes.

[rjwills28]

Thanks for the feedback @kasemir - much appreciated. Yes, that was a good catch - it needs to be able to remove the custom styling when the widget becomes enabled again without having to reload the page.

I have updated the code to do so. I've switched around some of the statements to make sure that we handle all cases. I've also surrounded the custom 'not_enabled' styling with comments in the css string signalling the start and end of that styling so that I can ensure that only this bit gets removed from the styling when the widget is enabled.

If you could take another look and let me know what you think that would be much appreciated.

[kasemir]

The approach with Styles.update(...) will add or remove a CSS class name, specifically not_enabled, from the getStyleClass() list. This is done with a contains check to prevent adding more than once.

This code includes node.setStyle(node.getStyle()+customCss) which would keep adding the same customCss each time it's called. Not sure if we know how often setDisabledLook gets called, might happen a few times and then the CSS text keeps growing.

Instead of patching CSS text, an overall better approach would be staying with a CSS class but change the definition of not_enabled.
So the code updated here would basically stay the same, Styles.update(node, Styles.NOT_ENABLED, !enabled);.
What you want to change is the definition of not_enabled which is in

phoebus/core/ui/src/main/resources/org/phoebus/ui/javafx/csstudio.css

Line 17 in 33bffe2

.not_enabled

It's loaded via Styles.setSceneStyle,

phoebus/core/ui/src/main/java/org/phoebus/ui/javafx/Styles.java

Line 23 in 33bffe2

public static void setSceneStyle(final Scene scene)

Maybe you can tweak that to update the definition of not_enabled?

[rjwills28]

Thanks again for the feedback.

Yes, I could add an if (!node.getStyle().contains(...)) then add the custom styling to make sure it doesn't add it again if it is already there.

I did look into trying to update the definition using a CSS class but couldn't find a way to do it or a way to load in a custom CSS file from somewhere outside of the project, which is ideally what we would want.

In the setSceneStyle() method the css string that is set is a path to the csstudio.css file built into the Phoebus project jar, i.e.:

jar:file:/home/software/phoebus/phoebus-product/target/lib/core-ui-5.0.3-SNAPSHOT.jar!/org/phoebus/ui/javafx/csstudio.css

and so I can't seem to get direct access to the actual CSS definition at this point to be able to modify it...

[kasemir]

In setSceneStyle, we add the path to the bundled csstudio.css.
As you noted, it's a resource URL.

THere's nothing that prevents adding another file:/path/to/my/site/adjustments.css.
You could get that URL, be it a file: or http, from a new "site_styles" preference.

In there, you can re-define the not_enabled style class, or leave it at the default.

This would allow each site to overwrite more than just not_enabled.
Surely everybody has their own preferred design for the tracker_handle, so that would now be up for local adjustments.

[rjwills28]

Hi @kasemir - thanks for the guidance. I've been able to get this working as you described. Not sure why I couldn't before - perhaps I was trying to load it in the wrong place.

I've reverted the previous changes and added a new property to the UI core package to allow users to add a custom CSS file.

The only other question I have is do we want any error checking when adding the CSS file to the stylesheet? The code does not throw an exception if the file added to the stylesheet does not actually exist. It is essentially just ignored and, for example, the default 'not_enabled' styling from csstudio.css is just used.

[kasemir]

Enthusiastic Yay!

As for a missing *.css style sheet, on one hand it seems to be an industry standard to ignore them. My web browser doesn't quit because of a missing style sheet, there's no popup, you need to dig real deep to notice.

But I think we should have a warning in the log.
Brings up the question how to check... In #3466 and earlier related issues, we tried to optimize via for example new URL(...).openConnection().connect() but that would connect fine even for cases where the document doesn't exist. Really need to check via new URL(...).openStream() to see if we can read the document. It's somewhat expensive to always read the document, we should not do that inside setSceneStyle each time that's called.
Maybe check once inside the static org.phoebus.ui.Preferences initializer:

    static
    {
        AnnotatedPreferences.initialize(Preferences.class, "/phoebus_ui_preferences.properties");

        // Check once if custom style sheet is accessible
        if (! custom_css_styling.isBlank())
        {
             try
             {
                 new URL(custom_css_styling).openStream().close();
             }
             catch (Exception ex)
             {
                    logger.log(Level.WARNING, custom_css_styling + " is inaccessible", ex);
                    custom_css_styling = "";
             }
     }

[rjwills28]

Ah that's nice - thanks for the code snippet. I have implemented that as suggested.

[rjwills28]

Resolved merge conflicts so this should be ready for re-review and merging if all are happy.