On Thu, 11 Aug 2022 23:14:39 GMT, Kevin Rushforth <[email protected]> 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

Reply via email to