RFR: 8311895: CSS Transitions [v16]

Michael Strauß mstrauss at openjdk.org
Fri May 24 11:18:36 UTC 2024


On Fri, 24 May 2024 10:02:44 GMT, drmarmac <duke at 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


More information about the openjfx-dev mailing list