RFR: 8290844: Add Skin.install() method [v2]
Marius Hanl
mhanl at openjdk.org
Thu Aug 11 14:08:37 UTC 2022
On Thu, 11 Aug 2022 13:55:10 GMT, Marius Hanl <mhanl at openjdk.org> 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
More information about the openjfx-dev
mailing list