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

Marius Hanl mhanl at openjdk.org
Fri Aug 12 05:06:27 UTC 2022


On Thu, 11 Aug 2022 23:14:39 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/e1057077...647ecd6c
>
> modules/javafx.controls/src/main/java/javafx/scene/control/Skinnable.java line 43:
> 
>> 41:      * It listens and responds to changes in state in a {@code Skinnable}.
>> 42:      * <p>
>> 43:      * There is typically a one-to-one relationship between a {@code Skinnable} and its
> 
> When isn't it a 1-to-1 relationship? Is it just for the special case of `PopupControl`?

Yes. All Skins derived by SkinBase must have a 1:1 relationship, while `getSkinnable()` for `PopupControl` is not really used.

>From my other comment:
It is a bit weird as Popups are not using the `getSkinnable` method it looks like. And also the `getSkinnable` method javadoc states: Gets the Skinnable to which this Skin is assigned. which is not true for the ComboBox Popup (as it sets the ComboBox as the skinnable) - while it makes sense that the ComboBox is set as styleable parent.

And:
So it is even more weird:
`getSkinnable()` is always called in Control skins, `getNode()` is only called in `Control.getSkinNode()`, PaginationSkin and TableColumnHeader (so not very much).
`getNode()` is always called for Popups (no `getSkinnable()` is called).

The only exception is the internal class `InputFieldSkin`, where the node is an 'inner TextField' and the skinnable the control where it is installed on.

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

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


More information about the openjfx-dev mailing list