RFR: 8311895: CSS Transitions [v2]

[Michael Strauß]

On Mon, 31 Jul 2023 12:16:05 GMT, John Hendrikx <jhendrikx at 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/TransitionDefinitionCssMetaData.java line 63:
> 
>> 61:     private static final Duration[] DURATION_ZERO = new Duration[] { Duration.ZERO };
>> 62: 
>> 63:     private static final Interpolator[] INTERPOLATOR_EASE = new Interpolator[] { InterpolatorConverter.CSS_EASE };
> 
> Careful, the array contents could still be modified

That's true. However, `CssParser` is built around array-based sequences (see `StringConverter.SequenceConverter`), and changing these arrays to lists would seem out of place for this part of the codebase. What do you think?

> modules/javafx.graphics/src/main/java/javafx/animation/Interpolator.java line 319:
> 
>> 317:      * The output time value is determined by the {@link StepPosition}.
>> 318:      *
>> 319:      * @param intervals the number of intervals in the step interpolator
> 
> minor: When I see a plural like `intervals` (or `employees`) I think of a list of objects. Perhaps `intervalCount` would be better?

Doesn't sound better to me, but I'll defer to what most people feel is best.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/870#discussion_r1279700471
PR Review Comment: https://git.openjdk.org/jfx/pull/870#discussion_r1279703051