> I think that the CSS reference should be generated from the source
code, which is something the annotation processor can do.
Surely, you don't suggest we should change the CSS ref?
https://openjfx.io/javadoc/21/javafx.graphics/javafx/scene/doc-files/cssref.html
I subscribe to a school of thought that requires specifications
written by humans for humans. The CSS reference is, in my opinion, an
authoritative source, so I would be against developing any tooling
that generates it from the source code.
-andy
*From: *Nir Lisker <nlis...@gmail.com>
*Date: *Thursday, December 7, 2023 at 01:54
*To: *Andy Goryachev <andy.goryac...@oracle.com>
*Cc: *Michael Strauß <michaelstr...@gmail.com>, openjfx-dev
<openjfx-dev@openjdk.org>
*Subject: *Re: [External] : Re: Reflective discovery of styleable
properties
- I would recommend against the scanner figuring out the property
name: the property > names are codified by the CSS reference which
serves as a normative document in this case
I think that the CSS reference should be generated from the source
code, which is something the annotation processor can do.
- isSettable() logic might be more complex than (prop == null &&
!(prop.isBound)), see Node.translateXProperty. How would that work
with annotations?
From what John said, you can fall back to the CSSMetaData way if the
defaults are not good for you.
On Wed, Dec 6, 2023 at 6:21 PM Andy Goryachev
<andy.goryac...@oracle.com> wrote:
I also like this idea very much. Still needs a reflective
scanner, but it's far more easier to understand and use.
A couple of comments/questions:
- I would recommend against the scanner figuring out the property
name: the property names are codified by the CSS reference which
serves as a normative document in this case
- isSettable() logic might be more complex than (prop == null &&
!(prop.isBound)), see Node.translateXProperty. How would that
work with annotations?
What do you think?
-andy
*From: *Nir Lisker <nlis...@gmail.com>
*Date: *Wednesday, December 6, 2023 at 02:37
*To: *Andy Goryachev <andy.goryac...@oracle.com>
*Cc: *Michael Strauß <michaelstr...@gmail.com>, openjfx-dev
<openjfx-dev@openjdk.org>
*Subject: *[External] : Re: Reflective discovery of styleable
properties
I thought about the option of reflection, but I opted to propose
annotations instead. The following is my reasoning.
Firstly, reflection is very magic-y. The author of the class has
no indication of what side effects happen due to the code they
write, the output (css handling in this case) comes out of nowhere
from their perspective. As with other reflection cases, it is a
"pull" rather than "push" approach - you don't write what should
happen, you let someone else decide that. For writers of
skin/control classes, this means that they need to know exactly
what constitutes a hook for the reflection mechanism, or face
surprises. There is no compile time check that tells you whether
you have declared your styleable property properly or not (without
an external ad-hoc checker).
We do this somewhat with properties - any method of the form
"...property()" gets special treatment, but this is for the docs.
I don't think we have code that depends on this other than in tests.
Secondly, the proposed mechanism depends on the runtime type, not
the declared type. As a user, I see no indication in the API
whether a property is styleable or not. This is also (what I would
consider) a problem with the current state. When I thought about
using reflection to solve this, I at least thought to specify the
declared type of the property as styleable, like
StyleableBooleanProperty instead of BooleanProperty (restricting
the returned type is backwards compatible). A downside of this is
that it gives access to the methods of StyleableProperty, which
are not useful for the user, I think, but maybe someone has a use
for them.
Thirdly, maybe I want to declare a styleable property not to be
used automatically. I can't think off the top of my head when I
would want to do that, but I'm also not a heavy css user. Are we
sure that just initializing a property with a styleable runtime
type should *always* be caught by this process?
To compare, annotations have the following benefits:
Firstly, they are declarative, which means no surprises for the
class author (WYSIWYG). This also allows more flexibility/control
over which properties get special treatment via an opt-in mechanism.
Secondly, They can also be displayed in the JavaDocs
(via @Documented) with their assigned values. For example, the
padding property of Region can be annotated
with @Styleable(property="-fx-padding"), informing the user both
that this value can be set by css, and how to do it.
Interestingly, the annotation doesn't need to be public API to be
displayed, so we are not bound by contracts.
In terms of similarities:
In both the reflection and the annotation proposals, the steps are:
1. Create styleable properties.
2. That's it.
It's just that step 1 also adds an annotation to the creation of
the property (which was/is a 2-step process anyway, declaring the
property and its css metadata).
Annotations also require a processor to read the data from their
values and target (the field/method). This is a bit of work, but
Michael's CssMetaDataCache class is basically that - read the data
from the class (via reflection or annotations) and store it in a
map. The logic should be the same, just the method to obtain the
data is different. Both, as a result, have the benefits of
handling control/skin combinations (what I mentioned in the point
"Usable both in controls and in skins (or other classes)").
The benefit of co-locating the property and its css metadata in
the class itself also remains.
To summarize, both approaches eliminate all the clutter of writing
styleable properties (John, will you like to create styleable
properties now? [1] :) ), both apply the flexibility of caching
per class, both allow better structuring of the class, but they
read the properties differently and have a different level of
declarativness.
[1]
https://mail.openjdk.org/pipermail/openjfx-dev/2023-December/044010.html
On Tue, Dec 5, 2023 at 11:21 PM Andy Goryachev
<andy.goryac...@oracle.com> wrote:
I like the idea.
I wonder if it is possible to reduce the amount of boilerplate
code? For example, a CssMetaData can have a
setGetter(Function<S, StyleableProperty<V>> getter)
method which supplies the property reference? This way
CssMetaData.isSettable(Node) and
CssMetaData.getStyleableProperty(Node) can be implemented in
the base class (there are more complicated cases, so perhaps
setIsSettable(Predicate<Node>) would also be required).
Example:
CssMetaData.<ControlExample,Font>of("-fx-font",
Font.getDefault(), (n) -> n.font)
Just a thought. What do you think?
-andy
*From: *openjfx-dev <openjfx-dev-r...@openjdk.org> on behalf
of Michael Strauß <michaelstr...@gmail.com>
*Date: *Sunday, December 3, 2023 at 22:02
*To: *openjfx-dev <openjfx-dev@openjdk.org>
*Subject: *Reflective discovery of styleable properties
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
<https://urldefense.com/v3/__https:/github.com/openjdk/jfx/pull/1299__;!!ACWV5N9M2RV99hQ!K7nDyvMP0PzEOLu-h9yGCoHIoSnny6LJ5acSISP81wBjJjP2z4VcDA6CIMU_Wvzxv2QJgPTsB6F9wtnzMK97$>
