Re: RFR: 8311895: CSS Transitions [v22]

2024-06-05 Thread Kevin Rushforth
On Wed, 5 Jun 2024 21:47:21 GMT, Michael Strauß  wrote:

>> modules/javafx.graphics/src/main/docs/javafx/scene/doc-files/cssref.html 
>> line 704:
>> 
>>> 702: 
>>> 703: >> scope="row">transition‑property
>>> 704:   [ none | all | # 
>>> ]
>> 
>> Why is there a literal `#` after `` (and others below)? It 
>> shows up when viewing the doc.
>
> The `#` indicates that one or more of the preceding entity can appear in a 
> comma-separated list.
> Contrast that with `*` (used in other places in cssref.html), which indicates 
> that zero or more of the preceding entity can appear (but doesn't prescribe a 
> comma-separated list).
> 
> See: https://www.w3.org/TR/css-values-4/#component-multipliers

Thanks. That makes sense then.

-

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


Re: RFR: 8311895: CSS Transitions [v22]

2024-06-05 Thread Michael Strauß
On Wed, 5 Jun 2024 17:43:46 GMT, Kevin Rushforth  wrote:

>> Michael Strauß has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 62 commits:
>> 
>>  - Merge branch 'refs/heads/master' into feature/css-transitions
>>  - document css parser limitation
>>  - documentation change
>>  - added documentation
>>  - address review comments
>>  - Merge branch 'refs/heads/master' into feature/css-transitions
>>  - extract magic string to named constant
>>  - use existing property in test
>>  - fixed documentation
>>  - Merge branch 'master' into feature/css-transitions
>>  - ... and 52 more: https://git.openjdk.org/jfx/compare/cf09d6f1...10c06a97
>
> modules/javafx.graphics/src/main/docs/javafx/scene/doc-files/cssref.html line 
> 704:
> 
>> 702: 
>> 703: > scope="row">transition‑property
>> 704:   [ none | all | # 
>> ]
> 
> Why is there a literal `#` after `` (and others below)? It 
> shows up when viewing the doc.

The `#` indicates that one or more of the preceding entity can appear in a 
comma-separated list.
Contrast that with `*` (used in other places in cssref.html), which indicates 
that zero or more of the preceding entity can appear (but doesn't prescribe a 
comma-separated list).

See: https://www.w3.org/TR/css-values-4/#component-multipliers

-

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


Re: RFR: 8311895: CSS Transitions [v22]

2024-06-05 Thread Kevin Rushforth
On Fri, 31 May 2024 15:09:38 GMT, Michael Strauß  wrote:

>> Implementation of [CSS 
>> Transitions](https://www.w3.org/TR/css-transitions-1/).
>> 
>> ### Future enhancements
>> CSS transition support for backgrounds and borders: #1471 
>> 
>> ### 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 62 commits:
> 
>  - Merge branch 'refs/heads/master' into feature/css-transitions
>  - document css parser limitation
>  - documentation change
>  - added documentation
>  - address review comments
>  - Merge branch 'refs/heads/master' into feature/css-transitions
>  - extract magic string to named constant
>  - use existing property in test
>  - fixed documentation
>  - Merge branch 'master' into feature/css-transitions
>  - ... and 52 more: https://git.openjdk.org/jfx/compare/cf09d6f1...10c06a97

The docs look good. I left one comment on the doc images and one question on 
the cssref.html changes. In parallel, I'll look at the CSR.

modules/javafx.graphics/src/main/docs/javafx/scene/doc-files/cssref.html line 
704:

> 702: 
> 703:  scope="row">transition‑property
> 704:   [ none | all | # 
> ]

Why is there a literal `#` after `` (and others below)? It shows 
up when viewing the doc.

-

PR Review: https://git.openjdk.org/jfx/pull/870#pullrequestreview-2097400072
PR Review Comment: https://git.openjdk.org/jfx/pull/870#discussion_r1628176431


Re: RFR: 8311895: CSS Transitions [v22]

2024-06-04 Thread Kevin Rushforth
On Fri, 31 May 2024 15:09:38 GMT, Michael Strauß  wrote:

>> Implementation of [CSS 
>> Transitions](https://www.w3.org/TR/css-transitions-1/).
>> 
>> ### Future enhancements
>> CSS transition support for backgrounds and borders: #1471 
>> 
>> ### 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 62 commits:
> 
>  - Merge branch 'refs/heads/master' into feature/css-transitions
>  - document css parser limitation
>  - documentation change
>  - added documentation
>  - address review comments
>  - Merge branch 'refs/heads/master' into feature/css-transitions
>  - extract magic string to named constant
>  - use existing property in test
>  - fixed documentation
>  - Merge branch 'master' into feature/css-transitions
>  - ... and 52 more: https://git.openjdk.org/jfx/compare/cf09d6f1...10c06a97

It looks like this is getting close to being ready, and would be a good feature 
to get into JavaFX 23. I'll take a look at the specification changes in the PR, 
and review the CSR.

-

PR Comment: https://git.openjdk.org/jfx/pull/870#issuecomment-2148395938


Re: RFR: 8311895: CSS Transitions [v22]

2024-05-31 Thread Michael Strauß
> 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 62 commits:

 - Merge branch 'refs/heads/master' into feature/css-transitions
 - document css parser limitation
 - documentation change
 - added documentation
 - address review comments
 - Merge branch 'refs/heads/master' into feature/css-transitions
 - extract magic string to named constant
 - use existing property in test
 - fixed documentation
 - Merge branch 'master' into feature/css-transitions
 - ... and 52 more: https://git.openjdk.org/jfx/compare/cf09d6f1...10c06a97

-

Changes: https://git.openjdk.org/jfx/pull/870/files
  Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=870&range=21
  Stats: 4786 lines in 47 files changed: 4743 ins; 4 del; 39 mod
  Patch: https://git.openjdk.org/jfx/pull/870.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/870/head:pull/870

PR: https://git.openjdk.org/jfx/pull/870