On Wed, 10 Aug 2022 22:14:53 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> - added Skin.install()
>> - javadoc changes for Skinnable.setSkin(Skin)
>>
>> no code changes for Skinnable.setSkin(Skin) yet.
>
> Andy Goryachev has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8290844: javadoc
>From a purist perspective, I love this change :) But ..
jumping to the impact analysis (and playing devil's advocate, a bit :)
> The changes should be fairly trivial. Only a subset of skins needs to be
> refactored, and the refactoring itself is trivial.
optimistic 😉 On the bright side: that subset are the harder ones, some being in
a hierarchy of skins where every level might do its own evil .. so we can eat
our own dog food (dealing with the added life-cyle).
> The new API is backwards compatible with the existing code, the
> customer-developed skins can remain unchanged (thanks to default
> implementation).
hmm ... probably I'm missing something but I don't see how that would be the
case for the subset of core skins that have to be refactored and moved their
installation into the install() method. Typically, custom skins do their own
stuff on top of what super did at the end of the constructor, assuming that
everything is fully installed. Depending on what exactly will be moved into
install, their behavior will be broken after the change.
Examples:
// prepending a listener
Consumer old = unregisterXXListener(someProperty);
registerXXListener(someProperty, consumer);
registerXXListener(someProperty, old);
// lookup a child added by super
Node child = control.lookup(..)
// replace singleton event handlers
control.setOnXX(...)
// probably more that I don't remember right now ..
-------------
PR: https://git.openjdk.org/jfx/pull/845
