On Fri, 24 May 2024 10:02:44 GMT, drmarmac <d...@openjdk.org> wrote:
>> Michael Strauß has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 53 commits:
>>
>> - Merge branch 'master' into feature/css-transitions
>> - update 'since' tags
>> - Fix javadoc error
>> - Change javadoc comment
>> - Merge branch 'master' into feature/css-transitions
>> - Discard redundant transitions in StyleableProperty impls
>> - Changes per review
>> - Merge branch 'master' into feature/css-transitions
>> - Merge branch 'master' into feature/css-transitions
>> - Add TransitionMediator
>> - ... and 43 more: https://git.openjdk.org/jfx/compare/aa9907a5...6614abb9
>
> modules/javafx.graphics/src/main/java/javafx/animation/Interpolator.java line
> 291:
>
>> 289: * All rise points are within the open interval (0..1).
>> 290: */
>> 291: BOTH,
>
> Looks like the docs for BOTH and NONE are swapped? (this would also affect
> the CSR)
Good catch! I've also updated the CSR.
> modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 9081:
>
>> 9079: TransitionDefinition transition = get(i);
>> 9080:
>> 9081: boolean selected =
>> "all".equals(transition.propertyName())
>
> Is "all" the same as `TransitionDefinitionCssMetaData.PROPERTY_ALL`, so the
> magic string can be avoided?
I've replaced `"all"` with a named constant in all relevant places.
> modules/javafx.graphics/src/test/java/test/javafx/scene/Node_transition_Test.java
> line 162:
>
>> 160: List<TransitionDefinition> transitions =
>> NodeShim.getTransitionDefinitions(node);
>> 161: assertEquals(3, transitions.size());
>> 162: assertTransitionEquals("-fx-background-color",
>> Duration.seconds(1), Duration.seconds(0.5), CSS_EASE, transitions.get(0));
>
> `node` is a `Rectangle`, which doesn't have a `background`property. Doesn't
> hurt this specific test, but maybe it's better to use an existing property.
I've changed the test to use `-fx-fill` instead.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/870#discussion_r1613307416
PR Review Comment: https://git.openjdk.org/jfx/pull/870#discussion_r1613307906
PR Review Comment: https://git.openjdk.org/jfx/pull/870#discussion_r1613308024
