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

Andy Goryachev angorya at openjdk.org
Fri Aug 12 17:53:33 UTC 2022


On Thu, 11 Aug 2022 23:03:49 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> 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 13 additional commits since the last revision:
>> 
>>  - 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
>>  - 8289397: added space
>>  - 8290844: skin.install
>>  - 8290844: whitespace
>>  - 8290844: validate input
>>  - 8290844: illegal argument exception
>>  - 8290844: illegal argument exception
>>  - ... and 3 more: https://git.openjdk.org/jfx/compare/1c437875...647ecd6c
>
> modules/javafx.controls/src/main/java/javafx/scene/control/Skin.java line 80:
> 
>> 78:      * previous skin has been uninstalled via its {@link #dispose()} method.
>> 79:      * This method allows a Skin to register listeners, add child nodes, set
>> 80:      * required properties and/or event handlers.
> 
> It might be good to add a sentence at the end saying: "The default implementation of this method does nothing".

thank you.

> modules/javafx.controls/src/main/java/javafx/scene/control/Skinnable.java line 59:
> 
>> 57:      * {@code Skin}, this method may check the return value of
>> 58:      * {@link Skin#getSkinnable()} against this Skinnable,
>> 59:      * and may throw an {@code IllegalArgumentException} if it is not the same.
> 
> Since most implementations of `Skinnable` will do this check, we probably want to strengthen this statement. I like the language you added to the `@throws` tag, which says that it will throw an exception if the Skin does not "correspond to" this Skinnable. Is there a way to work this in, by defining what we mean to "correspond to"?

@throws IllegalArgumentException if {@link Skin#getSkinnable()} returns a different {@code Skinnable}

is this better?

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

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


More information about the openjfx-dev mailing list