On Thu, 11 Aug 2022 13:55:10 GMT, Marius Hanl <[email protected]> wrote:
>> I like this idea.
>>
>> However, I am afraid that the impact of the check in PopupControl might be
>> greater than expected. In addition to your
>> ComboBoxPopupcontrol.createPopup() fix you provided earlier, there is a
>> similar situation with TooltipSkin in PopupcontrolTest:604:
>>
>>
>> Tooltip tooltip = new Tooltip("Hello");
>> TooltipSkin skin = new TooltipSkin(tooltip);
>> popup.setSkin(skin);
>>
>>
>> it is not entirely clear to me why a TooltipSkin is installed as a skin for
>> the popup instead of the tooltip.
>>
>> I think we should rather say that this 1:1 rule does not apply to
>> PopupControls.
>
> 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.
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
