Hi Andy,

see my comments below:


On Mon, Jan 23, 2023 at 10:33 PM Andy Goryachev
<andy.goryac...@oracle.com> wrote:
>
> Let me backtrack a bit and ask a very specific question: what is the problem 
> (or problems) you are trying to solve?
> I would like to understand if the problem exists (more on that below), does a 
> workaround exists or it is currently impossible to solve the problem using 
> the existing APIs, and relative importance of the problem.

While JavaFX knows about themes (after all, it ships with two
hard-wired themes), it doesn't offer a public API for custom themes.
That's a bit of a problem, especially since the built-in themes look
quite dated.
Now, what's a theme? It's a dynamic collection of user-agent
stylesheets with a bit of logic determining when and how individual
stylesheets are included in the CSS cascade.
In the CSS subsystem, JavaFX already supports application-wide
user-agent stylesheet collections (that's how the built-in themes are
implemented).
The public API seems like an afterthought: you set
Application.userAgentStylesheet to some magic string to select a
built-theme.

But of course, developers want custom themes. There are two basic workarounds:
1) Add author stylesheets to the Scene
2) Replace the built-in theme with a single new user-agent stylesheet

The first option can be used to extend or modify the built-in theme,
but it does so by changing the semantics of the new styles: author
stylesheets override user code, while user-agent stylesheets don't.
The second option retains the semantics of themes (allow user code to
override properties), but comes at the price of being incredibly
clunky:
* You can only specify a single stylesheet, which means there's no way
to create a custom theme comprised of many individual stylesheets.
* You can't modify existing themes. Once you set a non-magic
Application.userAgentStylesheet, you lose all stylesheets that came
with the built-in themes.
* You can't even copy the built-in theme stylesheets and modify the
files to your liking, because that way you'll lose all of the dynamic
features (for example, reacting to the high contrast scheme setting).

If JavaFX is to stay relevant, it must give developers the tools they
need to create visually pleasing applications. Dynamic themes are
absolutely required for that (can you imagine a modern application
that doesn't support dark mode?).



> For example, when speaking about themes and missing APIs, I would say that
>
> 1. Large scale stylesheet change at run time (i.e. going from Modena to 
> Caspian), or listing available stylesheets is probably not high on the list 
> of desired features.

Going from Modena to Caspian and back is an existing feature, so
that's already there. What do you mean by "listing available
stylesheets", and how is it relevant to this PR?



> 2. What is high on the list of desired features is ability to select color 
> theme (light/dark/hiContrast) at run time, possibly automatically by picking 
> up the OS preference is it exists.  Ideally, the color scheme would also pick 
> up other, user-defined colors from the OS and integrate this into the current 
> stylesheet (Modena).  Right now there is no public API to support that, as 
> far as I know, and no support in Modena.css

Since themes are an implementation detail right now, there's no public
API for anything of that sort.
The main premise of this PR is to promote themes to actual Java
classes. If you want to extend Modena, you simply extend the
javafx.scene.control.theme.ModenaTheme class and add your custom
stylesheets.
However, since modena.css is not designed to be extensible, the
ModenaTheme class also has very few extension points (basically just
an addFirst(String) and addLast(String) method to prepend or append
custom stylesheets).

If you wanted to make a version of Modena that supports dynamic
colors, you could do so by programmatically generating a small
stylesheet that only contains color definitions, and then simply swap
out this stylesheet in the ModenaTheme implementation whenever the
colors have changed. Of course, that also requires a bit of
refactoring of modena.css to remove the hard-coded colors. Note: it is
not a goal of this PR to refactor the built-in themes.



> 3. Another feature high on my wish list is ability to generate and apply CSS 
> changes at run time.  For example, reacting to the user selecting larger or 
> smaller font, more spacious or compact style, and so on.  Theoretically, this 
> is possible by generating a new stylesheet (it has to be a heavily modified 
> standard stylesheet) and feeding that to the Application using data: URL 
> protocol.

Sure, that's theoretically possible. But it's probably not a good
developer story to dump the entirety of an application's theme
stylesheets into a single data URI.

With this PR, you could support a compact mode by creating a custom
theme and exposing an API for that:

    public class MyCustomTheme implements StyleTheme {
        private final ObservableList<String> stylesheets =
            FXCollections.observableArrayList("controls.css");

        @Override
        public List<String> getStylesheets() {
             return stylesheets;
        }

        public void setCompactMode(boolean enabled) {
            if (enabled && stylesheets.size() == 1) {
                stylesheets.add("compact.css");
            } else if (!enabled && stylesheets.size() == 2) {
                stylesheets.remove(1);
            }
        }
    }



> 4. Since you do mention javafx.application.Platform.Preferences in your PR, I 
> fully agree that it is a good idea to expose platform and preferences, 
> especially since there are no public APIs for that.  This probably deserves 
> its own PR instead of being a part of #2.  But the main value, as I see it 
> (and I could be wrong) is that the platform preferences are incorporated into 
> the main stylesheet.

The Platform.Preferences API is required to promote Caspian and Modena
to first-class themes, since both themes support high-contrast colors
on Windows and need to continue doing so. This means they need a way
of knowing when to switch to high-contrast colors without hard-coding
that into JavaFX. Custom themes want to be able to support
high-contrast colors, too. :-)

I'm not sure if much is gained by separating those two things
(preferences and themes) into different PRs, as themes are probably
the main consumer of platform preferences. Designing platform
preferences in a vacuum might be harder than co-evolving it with
themes.

Reply via email to