On Mon, 2 Sep 2024 16:31:53 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
> I've been looking at `TransitionMediator`s and `TransitionTimer`s -- these > two classes seem to always go together. There is some nasty bookkeeping going > on that requires clearing a reference of the mediator. I've had some success > just eliminating the Mediator code and storing the timer directly. Yes, having an adapter between the styleable property implementation and the transition timer was an improvement that [you suggested](https://github.com/openjdk/jfx/pull/870#discussion_r1392551399); it seems like we've come full circle. Sure, clearing the mediator reference is not great, but it is a tiny and isolated part (as in, this implementation detail doesn't leak out of the class in which it is used). I wouldn't call this a serious problem of the design. > To make reversal detection work, you may need to pass something more to > `TransitionTimer.run` -- however, see Q1 -- is this something FX has decided > to provide that is not part of CSS? Should we consider making this smarter > (Q2)? Reversal detection requires each styleable property implementation to be able to update their reversing-adjusted start value. This interaction is one of the purposes of `TransitionMediator`. Should we make this smarter as prescribed by the specification? I don't think so. CSS transitions is not the right tool if you need more than simple animated state transitions. I want to stress the fact that these are _CSS_ transitions, not general-purpose transitions that can be triggered programmatically. This means that, while in theory there are multi-state transitions, these _always_ involve multiple pseudo-class states (for example, scenarios involving both :hover and :pressed). It is not possible to accumulate pseudo-class states in a way that would be similar to your Q2 scenario, where you programmatically set different values. > I could imagine the reversal detection should be working with a start value > that only resets on animation completion (ie. when `tt` is set back to > `null`). The reversal should then be calculated based on how much time we've > been animating already since `tt` became non-null (transition time minus time > spent animating already). If this is negative, then animate normally. Is it > positive, then shorten the transition. I don't think this would support a scenario where a transition is interrupted many times in a row, for example by moving the cursor across a button repeatedly, such that the :hover state is set, unset, then set again, and so on, and none of the transitions are allowed to run to completion. Pretty soon, the time already spent animating will exceed the total time of any single transition, which will prevent faster reversing for all future transitions. This is not what we would want to see. As for your ideas about refactoring the code to get rid of `TransitionMediator`, I'd like to understand a bit more context because this seems pretty random to me. Sure, I can imagine any number of implementations that would do the job, but what is your main motivation for rejecting the current implementation in favor for another approach? The purpose of `TransitionMediator` and `TransitionTimer` seem sufficiently clear-cut to me, and the implementation is reasonably clean given the complex problem being solved. I don't see an _obvious_ shortcoming of the current implementation. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1522#issuecomment-2325124450
