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