[External] : Re: Reflective discovery of styleable properties
Kevin Rushforth
kevin.rushforth at oracle.com
Thu Dec 7 16:07:23 UTC 2023
I haven't looked at this proposal yet, but there are some interesting
ideas here worth further discussion.
I just wanted to weigh in on the cssref piece. As Andy indicated,
cssref.html is the normative specification for JavaFX CSS properties.
And no, it can't be (fully) auto-generated.
The only way I could imagine tooling being involved is to auto-generate
the individual property tables for including in the html somehow (which
would need some sort of javadoc-inspired facility to add comments in the
table, etc). And while an interesting concept, it would likely be a
rather large effort for relatively little gain.
-- Kevin
On 12/7/2023 7:48 AM, Andy Goryachev wrote:
>
> > 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 <nlisker at gmail.com>
> *Date: *Thursday, December 7, 2023 at 01:54
> *To: *Andy Goryachev <andy.goryachev at oracle.com>
> *Cc: *Michael Strauß <michaelstrau2 at gmail.com>, openjfx-dev
> <openjfx-dev at 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.goryachev at 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 <nlisker at gmail.com>
> *Date: *Wednesday, December 6, 2023 at 02:37
> *To: *Andy Goryachev <andy.goryachev at oracle.com>
> *Cc: *Michael Strauß <michaelstrau2 at gmail.com>, openjfx-dev
> <openjfx-dev at 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.goryachev at 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-retn at openjdk.org> on behalf
> of Michael Strauß <michaelstrau2 at gmail.com>
> *Date: *Sunday, December 3, 2023 at 22:02
> *To: *openjfx-dev <openjfx-dev at 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$>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20231207/3a27a8f4/attachment-0001.htm>
More information about the openjfx-dev
mailing list