On Mon, 31 Jul 2023 12:29:28 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> Michael Strauß has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Make TransitionEvent final
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/css/TransitionTimer.java
> line 126:
>
>> 124: * @param forceStop if {@code true}, the timer is stopped
>> unconditionally
>> 125: * @return {@code true} if the timer was cancelled or {@code timer}
>> is {@code null},
>> 126: * {@code false} otherwise
>
> minor:
> Suggestion:
>
> * @return {@code true} if the timer was cancelled, or {@code timer} is
> {@code null},
> * otherwise {@code false}
I don't see any real difference here, but then again I'm not a native speaker.
I'll defer to public opinion.
> modules/javafx.graphics/src/main/java/javafx/css/StyleableBooleanProperty.java
> line 80:
>
>> 78: setValue(v);
>> 79: }
>> 80:
>
> I'm not sure how I feel about this; the changes made don't really seem to
> belong here.
>
> The `StyleableXXXProperty` classes are convenient helpers, but all that
> matters is that they are `StyleableProperty` implementations. There is no
> requirement to use the helpers. So what happens when a property defines
> their own helper, or implements `StyleableProperty` directly? Or if classes
> are refactored at some point and they decide to stop using these helpers?
I don't see how I can make CSS transitions work without some property-specific
implementation details. You're right that there is no requirement to use these
classes. An easy solution would be to just document that CSS transitions are
only supported for `StyleableXYZProperty` implementations.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/870#discussion_r1279727042
PR Review Comment: https://git.openjdk.org/jfx/pull/870#discussion_r1279725899
