RFR: 8290844: Add Skin.install() method [v4]

Kevin Rushforth kcr at openjdk.org
Wed Sep 7 20:58:57 UTC 2022


On Fri, 12 Aug 2022 18:15:37 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> - added Skin.install()
>> - javadoc changes for Skinnable.setSkin(Skin)
>> 
>> no code changes for Skinnable.setSkin(Skin) yet.
>
> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8290844: review comments

I left feedback on the API docs.

Once we get to the implementation review, this will need some unit tests.

modules/javafx.controls/src/main/java/javafx/scene/control/PopupControl.java line 267:

> 265: 
> 266:             // let the new skin modify this control
> 267:             if(skin != null) {

Minor: add a space after the `if`

modules/javafx.controls/src/main/java/javafx/scene/control/Skin.java line 35:

> 33:  * A user interface control is abstracted behind the {@link Skinnable} interface.
> 34:  * <p>
> 35:  * A Skin implementation should generally avoid modifying its control outside of

I think this is a good start. As discussed elsewhere, additional implementation notes for Skin implementers would be helpful.

modules/javafx.controls/src/main/java/javafx/scene/control/Skin.java line 82:

> 80:      * required properties and/or event handlers.
> 81:      * <p>
> 82:      * The default implementation of this method does nothing.

You need to add `@since 20` here.

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

> 37: public interface Skinnable {
> 38:     /**
> 39:      * Skin is responsible for rendering this {@code Skinnable}. From the

Suggestion:

"The Skin that is responsible for..."

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

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


More information about the openjfx-dev mailing list