It looks good, and IMHO would be a major clean up, and would simplify creating stylable properties for users tremendously.

I see you added a public API in CssMetaData, but I think you can just leave that private in CssMetaDataCache.  Users won't need it, as they should no longer be overriding getCssMetaData.  Old implementations are backwards compatible, so they won't need it either.

Any new Nodes/Controls need not (or should not) provide the static getClassCssMetaData or override getCssMetaData.

+1

--John

On 04/12/2023 07:02, Michael Strauß wrote:
Following up the discussion around the CssMetaData API, I'd like to
chime in with yet another idea. To recap, here's Nir's summary of the
current API [0]:

"Let's look at what implementation is required from a user who wants
to write their own styleable control:
1. Create styleable properties.
2. Create a list of these properties to be passed on.
3. Create a public static method that returns the concatenation of
this list with the one of its parent. (This method happens to be
poorly documented, as mstr said.)
4. Create a public non-static method that calls the static method in a
forced-override pattern because otherwise you will be calling the
wrong static method. (This method's docs seem to be just wrong because
you don't always want to delegate to Node's list.)"


I think this could reasonably be replaced with the following
implementation requirements:
1. Create styleable properties.
2. That's it.

Let's look at what we're actually trying to do: create a list of
CSS-styleable property metadata of a class. But we can easily do that
without all of the boilerplate code.

When ´Node.getCssMetaData()` is invoked, all public methods of the
class are reflectively enumerated, and metadata is retrieved from
`Property` and `StyleableProperty` getters. This is a price that's
only paid once for any particular class (i.e. not for every instance).
The resulting metadata list is cached and reused for all instances of
that particular class.

As a further optimization, metadata lists are also cached and
deduplicated for Control/Skin combinations (currently every Control
instance has its own copy of the metadata list).

Another benefit of this approach is that the CssMetaData can now be
co-located with the property implementation, and not be kept around in
other parts of the source code file. Here's how that looks like when a
new "myValue" property is added to MyClass:

     StyleableDoubleProperty myValue =
             new SimpleStyleableDoubleProperty(this, "myValue") {

         static final CssMetaData<MyClass, Number> METADATA =
             new CssMetaData<MyClass, Number>(
                 "-fx-my-value",
                 SizeConverter.getInstance(),
                 USE_COMPUTED_SIZE) {
             @Override
             public boolean isSettable(MyClass node) {
                 return !node.myValue.isBound();
             }

             @Override
             public StyleableProperty getStyleableProperty(
                     MyClass node) {
                 return node.myValue;
             }
         };

         @Override
         public CssMetaData getCssMetaData() {
             return METADATA;
         }
     };

     public final DoubleProperty myValueProperty() {
         return myValue;
     }

It is not required to override the `getCssMetaData()` method, nor is
it required to redeclare a new static `getClassCssMetaData()` method.
It is also not required to manually keep the list of styleable
properties in sync with the list of CSS metadata.

I've prototyped this concept for the `Node`, `Region` and `Control` classes [1].

[0] https://mail.openjdk.org/pipermail/openjfx-dev/2023-December/044046.html
[1] https://github.com/openjdk/jfx/pull/1299

Reply via email to