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