Support global theme that applies to all new text areas

[psiinon]

We use RSyntaxTextArea in ZAP (https://github.com/zaproxy/zaproxy/) - so many thanks :) The user can set any Java LookAndFeel but we've recently added dark mode support as per https://github.com/zaproxy/zaproxy/issues/5542 and need to change our RSyntaxTextArea's to match. We're happy to check which LookAndFeel the user has chosen and then apply a suitable RSyntaxTextArea theme, but doing this for all text areas is a pain, especially as ZAP supports add-ons which can add new screens and dialogs, any of which could add new RSyntaxTextArea's. A global option which we could set when we set the LookAndFeel would be really useful.

[thc202]

We are happy to provide a PR, do you have a preference on how this should be implemented?

[vlsi]

I wonder if this could/should be resolved by overriding RTextArea#getUIClassID.

An alternative option is probably override TextAreaUI at the application side.

In other words: 1) install LaF 2) Get UI class for TextAreaUI 3) Install app class into UIManager

Then app-installed class can apply whatever it needs for all TextAreas by augmenting it in ...#createUI() method.

[thc202]

I wonder if this could/should be resolved by overriding RTextArea#getUIClassID.

Could you elaborate how that would help resolve this issue?

Then app-installed class can apply whatever it needs for all TextAreas by augmenting it in ...#createUI() method.

Applying a Theme requires that the object is (almost) completely initialised.

[vlsi]

Applying a Theme requires that the object is (almost) completely initialised

Component#updateUI is invoked when the component is fully initialized. For instance, updateUI is called when LaF is changed, so it looks like that should work.

[thc202]

Component#updateUI is invoked when the component is fully initialized.

The object (RSyntaxTextArea in this case) is not completely initialised when updateUI is called, attempting to apply a Theme would fail.

For instance, updateUI is called when LaF is changed, so it looks like that should work.

This issue is/was not about look and feel changes, in our case we just set it once.

[vlsi]

The object (RSyntaxTextArea in this case) is not completely initialised when updateUI is called, attempting to apply a Theme would fail.

Is this RSyntaxTextArea bug then?

What do you mean by not completely initialised?

[thc202]

Is this RSyntaxTextArea bug then?

I'd not consider it a bug, the object is still being initialised (i.e. this is being leaked/escaped).

What do you mean by not completely initialised?

Was using the definition from the JLS:

An object is considered to be completely initialized when its constructor finishes.

Which ensures the object is in the expected state, ready to be used. (Strictly it does need to wait for the constructor to finish (e.g. call init sometime before), but still probably not a good practise IMO.)

[vlsi]

Well, technically speaking, setUI is called before RSA#<init> finishes, however, that is a default Swing practice.

JComponent#setUI calls ui.installUI(this) which should configure UI representation, keymaps, etc, for the component.

[vlsi]

psiinon: We're happy to check which LookAndFeel the user has chosen and then apply a suitable RSyntaxTextArea theme, but doing this for all text areas is a pain, especially as ZAP supports add-ons which can add new screens and dialogs, any of which could add new RSyntaxTextArea's

I guess ZAP has other controls (e.g. text field, checkbox, and so on). Do you really intend to provide a PR for JTextField, JCheckBox and so on?

Why is RSyntaxTextArea any different?

A global option which we could set when we set the LookAndFeel would be really useful.

Swing API does have exactly this implemented: UIDefaults -> JComponent#getUIClassID -> ... It does work for all the other controls.

[bobbylight]

Another possibility is some sort of factory for RSTA instances, encouraging add-ons to call into that?

public static RSyntaxTextArea createTextArea() {
  RSyntaxTextArea textArea = new RSyntaxTextArea(80, 25);
  globalRstaTheme.apply(textArea);
  return textArea;
}

[vlsi]

Another possibility is some sort of factory for RSTA instances, encouraging add-ons to call into that?

Could you please compare that to new JLabel()? Apparently, addons do not need to use app-specific factories to create Labels :-/ That is why I guess RSTA should be themable without custom factories.

One more suggestion: how about RSTA picking a default theme class from UIDefaults?

In other words, the application would put the desired RSTA theme as UIManager.getLookAndFeelDefaults().put("RSyntaxTextArea.theme", ThemeObjectOrName") and then RSTA should use that theme when initializes. I guess that is the most convenient and the most typical Swing way of applying Look and Feel parameters.

[bobbylight]

Sounds like a good idea, short of redoing the Theme API to be better integrated into RSTA.

The best place to put the Theme application logic is in addNotify() since Theme includes properties that get applied to the sibling Gutter component in the parent RTextScrollPane, if it exists.

The new API would be like @vlsi proposes. At some point during initialization an application would set:

UIManager.getLookAndFeelDefaults().put("RSyntaxTextArea.theme", myTheme);

Every RSTA instance created would apply that theme. This would also affect any wrapping RTextEditorPanes and Gutters.

What's not great about this is if the application wants to change the default Theme at runtime (say to coordinate with a runtime Look and Feel change). Existing RSTA instances would not be aware of a UIManager property changing, so it would be up to the application and/or add-ons to listen for UIManager property change events (or use some other mechanism) to detect a new theme to apply, and to do so themselves.

This also has the nice property of being backwards compatible, not requiring me to create a new version of RSTA with an API that behaves in another way :)

Does that sound reasonable?

[vlsi]

What's not great about this is if the application wants to change the default Theme at runtime (say to coordinate with a runtime Look and Feel change). Existing RSTA instances would not be aware of a UIManager property changing,

Why do you think so?

Regular JTextArea (and all other Swing) components update their styles in response to javax.swing.JComponent#updateUI call.

In other words, RSTA should fetch the styles from UIDefaults when updateUI is called. When the application applies new look and feel, it would call updateUI (e.g. via javax.swing.SwingUtilities#updateComponentTreeUI), so all the components are updated to the new style.

Note: there's javax.swing.plaf.UIResource interaface.

If a theme that is assigned to RSTA does not implement UIResource, then RSTA should refrain from using a theme from UIDefaults. On the other hand, if the current RSTA theme implements UIResource (or null), then RSTA should override the theme with UIDefaults when updateUI happens.

The meaning there is users should be able to customize a theme for a given RSTA instance that would not be changed automatically when LaF changes.

[bobbylight]

The reason I said it's "not great" is because I suggested putting the theme setting code in addNotify(), not in updateUI(), so it wouldn't get automatically updated.

I'm aware of UIResource and use it, the issue here is RSTA doesn't store its Theme, rather the theme sets colors and fonts in a other structure and format that RSTA uses internally. Thus we have no "current" Theme check for implementing UIResource. If applying a theme in updateUI(), we'd have to blindly apply the new theme without changing the RSTA styling code or the app doing more work itself.

[vlsi]

I suggested putting the theme setting code in addNotify()

Ah, I see.

Thus we have no "current" Theme check for implementing UIResource

Frankly speaking, I think it is implementation details which you, as the maintainer, know better and which we, as the end-users, should not know/care.

You could save the current scheme. You could work with individual colors and check UIResource for individual colors/fonts. You could probably have other options.

As the end-user for RSTA, I would expect that RSTA can be styled as other Swing components: UIDefaults + updateUI(). I expect that RSTA should support UIResource as well. For instance, I expect there might be cases when a specific RSTA instance should not automatically update when LaF changes.

[bobbylight]

Those are fair things to ask, but at present RSTA isn't quite designed that way. Themes "apply" themseves by setting color/font properties into the RSTA instance's SyntaxScheme, which is a lower-level styling API (predates the Theme API by ages).

I could see work in a major release to implement your suggestions @vlsi as follows:

• Consolidate Theme and SyntaxScheme, likely by removing the latter
• Have the Theme be part of the RSTA API like SyntaxScheme is today - gettable/settable, etc. - rather than having it "apply" its colors and fonts into existing, other RSTA properties

This would likely be in a 4.0.0 release since it's a larger API change. Feel free to open that if you like - I have been meaning to start brainstorming ideas for a new "major" release.

@psiinon - Let me do some research into this. IMO a fair tradeoff for all points made in this thread would behave as follows:

• Some way to define a global "default" Theme
• This gets applied to every RSTA instance instantiated
• Bonus points if a runtime changing of the LookAndFeel re-checks and applies a new Theme (this isn't necessary in your case I believe but would be nice if it "just worked" this way)

If that sounds OK let me know - either I could work on it or anybody here could work on a PR for it. The lifecycle hooks like addNotify() and updateUI() seem like good places to start looking. I'm indifferent between using a UIDefault property or some static global like Theme.getDefault() (I know many consider stuff like that bad practice).

[vlsi]

Bonus points if a runtime changing of the LookAndFeel re-checks and applies a new Theme

Just in case: RTSA is used in Apache JMeter, and on-the-fly LAF change is a thing there.

addNotify()

Please, do not abuse addNotify.

[vlsi]

indifferent between using a UIDefault property or some static global like Theme.getDefault

UIDefault provides lazy evaluation (UIDefaults.LazyValue) and factory (UIDefaults.ActiveValue) for free, and it is a common Swing practice.

Apparently the current approach of Theme#apply(RTSA) does not work well when LaF is a question.