On Sun, 4 Aug 2024 15:32:10 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> I've taken a closer look, and I have the feeling that >> `ComponentTransitionable` is more of an indicator that the corresponding >> `StyleConverter` has implemented `convertBack`. I think noting this >> information closer to the source would therefore be a good idea. >> >> From what I can see, `applyComponentTransition` already falls back to >> calling `set(T)` if there is anything out of the ordinary (like missing CSS >> meta data or a missing StyleConverter). >> >> For both interpolation cases (`Interpolatable` and >> `ComponentTransitionable`) you are getting the CSS meta data. You can >> externalize that part. When you have the meta data, you can get the >> `StyleConverter` and check if it supports deconstruction (instead of having >> a marker on the final result). You can either: >> >> - put the marker on `StyleConverter` >> - have `convertBack` return an `Optional` or `null` if unsupported (there >> are already several `null` checks for missing CSS meta data or the missing >> converter, one more here seems not unreasonable). >> - have a method on `StyleConverter` that indicates its capabilities >> (true/false, `SUPPORTS_DECONSTRUCTION`, etc..) >> >> The advantage of this IMHO is that it keeps CSS concerns (component >> transitionable) in the CSS realm (with StyleConverter), while leaving simple >> classes that are just value holders (Border, Background) untouched. > > I don't quite like returning any particular value from `convertBack` as a > signal for "this style converter doesn't support deconstruction", so I've > made it more explicit with the marker interface > `StyleConverter.SupportsDeconstruction`. This removes the marker interface > `ComponentTransitionable` entirely. Thanks, I think having this with the StyleConverter makes more sense. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1703410832
