On Thu, 2 May 2024 08:40:28 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> Implementation of [CSS >> Transitions](https://gist.github.com/mstr2/c72f8c9faa87de14926978f517a6018a). >> >> ### Future enhancements >> CSS transitions requires all participating objects to implement the >> `Interpolatable` interface. For example, targeting `-fx-background-color` >> only works if all background-related objects are interpolatable: `Color`, >> `BackgroundFill`, and `Background`. >> >> In a follow-up PR, the following types will implement the `Interpolatable` >> interface: >> `LinearGradient`, `RadialGradient`, `Stop`, `Background`, `BackgroundFill`, >> `BackgroundImage`, `BackgroundPosition`, `BackgroundSize`, >> `BackgroundStroke`, `BorderWidths`, `CornerRadii`, `Insets`. >> >> ### Limitations >> This implementation supports both shorthand and longhand notations for the >> `transition` property. However, due to limitations of JavaFX CSS, mixing >> both notations doesn't work: >> >> .button { >> transition: -fx-background-color 1s; >> transition-easing-function: linear; >> } >> >> This issue should be addressed in a follow-up enhancement. > > 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 I did some testing of this PR and started looking into the implementation (see code comments). - Fade in / fade out with delay and various easing functions via the opacity property works as expected, , scaleX/scaleY also look good - Attempting to do background-color transitions doesn't work, I presume because it's not yet `Interpolatable` in this PR. Do we need to emit a warning in such a case? Right now the `transition` CSS code just seems to be ignored. - Same with the current limitation of not being able to mix shorthand and longhand notations, do we need a warning if it is attempted? - Generally this looks like a high quality PR, filling a long-standing gap. Currently available alternative solutions (e.g. using code-driven animations) are quite cumbersome and not easy to get right. 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) 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? 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. ------------- PR Review: https://git.openjdk.org/jfx/pull/870#pullrequestreview-2076453636 PR Review Comment: https://git.openjdk.org/jfx/pull/870#discussion_r1613220507 PR Review Comment: https://git.openjdk.org/jfx/pull/870#discussion_r1613227193 PR Review Comment: https://git.openjdk.org/jfx/pull/870#discussion_r1613233729
