RFR: 8350917: Allow parent nodes to provide CSS styleable properties for child nodes
Michael Strauß
mstrauss at openjdk.org
Sun Mar 16 19:11:02 UTC 2025
On Mon, 17 Feb 2025 01:15:37 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:
> 8350917: Allow parent nodes to provide CSS styleable properties for child nodes
modules/javafx.graphics/src/main/java/javafx/css/Styleable.java line 101:
> 99: * only for its direct children!
> 100: *
> 101: * @return an immutable list of CSS meta data, never {@code null}
Since we don't specify that `getCssMetaData()` should never return null, we should consider also not specifying it here. Alternatively, maybe we should add the non-null requirement to the existing `getCssMetaData()`.
modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java line 465:
> 463: }
> 464:
> 465: private record StylingContext(Node node, CalculatedValue font, StyleMap styleMap, Set<PseudoClass> pseudoClasses) {}
It might not be a good trade-off to create lots of transient objects on a hot path just to save a few arguments in the calling convention. This would be a nice improvement once we have value types in Java.
modules/javafx.graphics/src/main/java/javafx/scene/layout/Pane.java line 128:
> 126: @SuppressWarnings("unchecked")
> 127: StyleableObjectProperty<T> castProperty = (StyleableObjectProperty<T>) child.getProperties()
> 128: .computeIfAbsent(cssMetaData, k -> createChildConstraintProperty(child, cssMetaData));
Since we're on a hot path here, I'd like to see this work without unnecessary memory allocations:
1. Check `hasProperties()` before calling `getProperties()`.
2. Don't use `computeIfAbsent()` since it requires a capturing lambda.
modules/javafx.graphics/src/main/java/javafx/scene/layout/Pane.java line 140:
> 138: }
> 139:
> 140: String propertyName = name + " (parent property)";
I think a more sensible naming scheme would be something like "VBox.margin", which should be supplied as an argument and not generated on the fly.
modules/javafx.graphics/src/main/java/javafx/scene/layout/Pane.java line 234:
> 232: }
> 233:
> 234: if (value != null) {
How would the value ever be anything other than a `StyleableObjectProperty`, since `setChildConstraint()` always creates a `StyleableObjectProperty`?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1714#discussion_r1997679699
PR Review Comment: https://git.openjdk.org/jfx/pull/1714#discussion_r1997663655
PR Review Comment: https://git.openjdk.org/jfx/pull/1714#discussion_r1997683419
PR Review Comment: https://git.openjdk.org/jfx/pull/1714#discussion_r1997682667
PR Review Comment: https://git.openjdk.org/jfx/pull/1714#discussion_r1997684595
More information about the openjfx-dev
mailing list