On Thu, 20 Oct 2022 19:07:47 GMT, Andy Goryachev <[email protected]> wrote:

>> - added Skin.install()
>> - javadoc changes for Skinnable.setSkin(Skin)
>
> Andy Goryachev has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains 26 additional 
> commits since the last revision:
> 
>  - Merge remote-tracking branch 'origin/master' into 8290844.skin.install
>  - 8290844: review comments
>  - Merge remote-tracking branch 'origin/master' into 8290844.skin.install
>  - 8290844: javadoc
>  - Merge remote-tracking branch 'origin/master' into 8290844.skin.install
>  - 8290844: javadoc
>  - Merge branch 'openjdk:master' into 8290844.skin.install
>  - 8290844: unit tests
>  - Merge remote-tracking branch 'origin/master' into 8290844.skin.install
>  - 8290844: review comments
>  - ... and 16 more: https://git.openjdk.org/jfx/compare/9bab17bd...4aaf422b

The javadoc now looks good to me with two minor changes requested. Once you 
make those changes, go ahead and update the CSR and "Propose" it.

modules/javafx.controls/src/main/java/javafx/scene/control/Control.java line 
226:

> 224:      * <p>
> 225:      * To ensure a one-to-one relationship between a {@code Control} and 
> its {@code Skin},
> 226:      * setting the {@link #skinProperty() skin property} to a non-null 
> {@code Skin} first checks

This need not be a link, since it is already in the skin property. I recommend 
changing it to code style.

modules/javafx.controls/src/main/java/javafx/scene/control/Skinnable.java line 
46:

> 44:      * and its {@code Skin}. Every {@code Skin} maintains a back 
> reference to the
> 45:      * {@code Skinnable}.  When required, this relationship is enforced 
> inside {@link Skinnable#setSkin(Skin)} method
> 46:      * which checks the return value of {@link Skin#getSkinnable()} 
> against this {@code Skinnable},

As with other occurrences, I recommend changing `inside {@link 
Skinnable#setSkin(Skin)} method` to `when the skin is set`.

-------------

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

Reply via email to