Re: RFR: 8311895: CSS Transitions [v2]

2024-05-25 Thread Nir Lisker
On Mon, 31 Jul 2023 18:10:46 GMT, Michael Strauß wrote: >> 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 int

Re: RFR: 8311895: CSS Transitions [v2]

2024-03-18 Thread Michael Strauß
On Mon, 31 Jul 2023 13:44:28 GMT, John Hendrikx wrote: >> Michael Strauß has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Make TransitionEvent final > > Some early feedback, it's a lot of code :) I've implemented @hjohn's suggestion of a

Re: RFR: 8311895: CSS Transitions [v2]

2023-11-13 Thread Michael Strauß
On Mon, 31 Jul 2023 13:44:28 GMT, John Hendrikx wrote: >> Michael Strauß has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Make TransitionEvent final > > Some early feedback, it's a lot of code :) @hjohn Would you be willing to expand you

Re: RFR: 8311895: CSS Transitions [v2]

2023-08-01 Thread John Hendrikx
On Tue, 1 Aug 2023 17:02:33 GMT, Michael Strauß wrote: >> Yeah, I figured as much. I just don't like code that you can't get covered >> in a unit test, or that contradicts itself. As you can see, I was confused, >> is it a bug or a feature? Perhaps a comment then to indicate that it's >> in

Re: RFR: 8311895: CSS Transitions [v2]

2023-08-01 Thread John Hendrikx
On Tue, 1 Aug 2023 16:44:08 GMT, Michael Strauß wrote: >> I'm not a native either, but it flows a bit better IMHO. I checked `String` >> class, they do it like that there as well. > > In the `String` class, I find 10 usages of `{@code false} otherwise`, and 2 > usages of `otherwise {@code fals

Re: RFR: 8311895: CSS Transitions [v2]

2023-08-01 Thread Michael Strauß
On Tue, 1 Aug 2023 10:49:42 GMT, John Hendrikx wrote: >> The constructor ensures that any spelling of "ALL" is converted to the >> interned constant "all", which is important as we would otherwise need a >> more computationally expensive case-insensitive string comparison in >> `Node.Transitio

Re: RFR: 8311895: CSS Transitions [v2]

2023-08-01 Thread Michael Strauß
On Tue, 1 Aug 2023 13:46:15 GMT, John Hendrikx wrote: >> 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? > > It d

Re: RFR: 8311895: CSS Transitions [v2]

2023-08-01 Thread Michael Strauß
On Tue, 1 Aug 2023 13:47:19 GMT, John Hendrikx wrote: >> I don't see any real difference here, but then again I'm not a native >> speaker. I'll defer to public opinion. > > I'm not a native either, but it flows a bit better IMHO. I checked `String` > class, they do it like that there as well.

Re: RFR: 8311895: CSS Transitions [v2]

2023-08-01 Thread John Hendrikx
On Mon, 31 Jul 2023 18:20:34 GMT, Michael Strauß wrote: >> modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 8968: >> >>> 8966: >>> 8967: for (TransitionTimer timer : transitionTimers) { >>> 8968: if (timer.getProperty() == property) { >> >> minor: this prob

Re: RFR: 8311895: CSS Transitions [v2]

2023-08-01 Thread John Hendrikx
On Mon, 31 Jul 2023 18:16:19 GMT, Michael Strauß wrote: >> modules/javafx.graphics/src/main/java/javafx/css/TransitionEvent.java line >> 88: >> >>> 86: * @param eventType the event type >>> 87: * @param property the {@code StyleableProperty} that is targeted >>> by the transition >>>

Re: RFR: 8311895: CSS Transitions [v2]

2023-08-01 Thread John Hendrikx
On Mon, 31 Jul 2023 18:01:31 GMT, Michael Strauß wrote: >> modules/javafx.graphics/src/main/java/com/sun/scenario/animation/StepInterpolator.java >> line 73: >> >>> 71: } >>> 72: >>> 73: if (t >= 0 && step < 0) { >> >> `t >= 0` always holds, perhaps the original `t` was meant

Re: RFR: 8311895: CSS Transitions [v2]

2023-08-01 Thread John Hendrikx
On Mon, 31 Jul 2023 18:07:52 GMT, Michael Strauß wrote: >> 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

Re: RFR: 8311895: CSS Transitions [v2]

2023-08-01 Thread John Hendrikx
On Mon, 31 Jul 2023 18:29:19 GMT, Michael Strauß wrote: >> modules/javafx.graphics/src/main/java/com/sun/javafx/css/TransitionDefinition.java >> line 54: >> >>> 52: */ >>> 53: public TransitionDefinition(String propertyName, Duration duration, >>> 54: Du

Re: RFR: 8311895: CSS Transitions [v2]

2023-07-31 Thread Michael Strauß
On Mon, 31 Jul 2023 12:29:28 GMT, John Hendrikx 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/TransitionTimer.java > line

Re: RFR: 8311895: CSS Transitions [v2]

2023-07-31 Thread Michael Strauß
On Mon, 31 Jul 2023 12:04:19 GMT, John Hendrikx 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/TransitionDefinition.java >

Re: RFR: 8311895: CSS Transitions [v2]

2023-07-31 Thread Michael Strauß
On Mon, 31 Jul 2023 13:39:06 GMT, John Hendrikx 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/javafx/scene/Node.java line 8968: > >> 8966:

Re: RFR: 8311895: CSS Transitions [v2]

2023-07-31 Thread Michael Strauß
On Mon, 31 Jul 2023 13:33:41 GMT, John Hendrikx 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/javafx/css/TransitionEvent.java line 88: > >>

Re: RFR: 8311895: CSS Transitions [v2]

2023-07-31 Thread Michael Strauß
On Mon, 31 Jul 2023 12:16:05 GMT, John Hendrikx 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/TransitionDefinitionCssMetaD

Re: RFR: 8311895: CSS Transitions [v2]

2023-07-31 Thread Michael Strauß
On Mon, 31 Jul 2023 12:39:26 GMT, John Hendrikx 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/TransitionDefinition.java >

Re: RFR: 8311895: CSS Transitions [v2]

2023-07-31 Thread Michael Strauß
On Mon, 31 Jul 2023 12:56:10 GMT, John Hendrikx 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/scenario/animation/StepInterpolator.jav

Re: RFR: 8311895: CSS Transitions [v2]

2023-07-31 Thread John Hendrikx
On Mon, 31 Jul 2023 00:09:38 GMT, Michael Strauß wrote: >> Implementation of [CSS >> Transitions](https://gist.github.com/mstr2/c72f8c9faa87de14926978f517a6018a). >> >> ### Example >> >> .button { >> -fx-background-color: dodgerblue; >> } >> >> .button:hover { >> -fx-background-color:

Re: RFR: 8311895: CSS Transitions [v2]

2023-07-30 Thread Michael Strauß
> Implementation of [CSS > Transitions](https://gist.github.com/mstr2/c72f8c9faa87de14926978f517a6018a). > > ### Example > > .button { > -fx-background-color: dodgerblue; > } > > .button:hover { > -fx-background-color: red; > -fx-scale-x: 1.1; > -fx-scale-y: 1.1; > > transi