On Tue, 4 Oct 2022 14:39:47 GMT, Jeanette Winzenburg <[email protected]>
wrote:
>>> In fact, it's so easy that most built-in skins do it (and these were
>>> created by the developers of JavaFX itself, so obviously it's a major
>>> problem if even those people get it wrong so often).
>>>
>>
>> well, just guessing games here but: mine is that they didn't put too many
>> thoughts into the possibility of replacing a skin and if they did, they
>> actively prevented it - as seen in the implementation of
>> TextAreaSkin.dispose when we started the current cleanup round:
>>
>> /** {@inheritDoc} */
>> @Override public void dispose() {
>> super.dispose();
>>
>> if (behavior != null) {
>> behavior.dispose();
>> }
>>
>> // TODO Unregister listeners on text editor, paragraph list
>> throw new UnsupportedOperationException();
>> }
>
>> You are right, @kleopatra - this executeOnceWhenPropertyIsNonNull() is
>> installing a listener. perhaps we should add a similar functionality to
>> LambdaMultiplePropertyChangeListenerHandler and actually install a
>> WeakListener instead.
>
> - yes, we need weakListeners (to wait for a not-null value to add the event
> handler) _and_ remove both the weakListener and the event handlers in dispose
> - adding functionality to the LambdaHandler sounds like a good idea, but
> needs some thinking and should be done in a separate issue which also adds
> delegate methods to expose the new functionality in SkinBase for use in sub
> classes
>
> For this issue, you might simply inline the utility method and register the
> listeners using skinbase api (didn't try, it's your issue :), see
> TableRowSkin for a similar pattern
>
> This conditional adding of handlers/listeners is needed if we have to support
> path properties (also f.i. when listening to selectedXX in any of the skins
> with selectionModels as properties). Which basically should be supported by
> base - but probably without any chance to ever get them ;)
ListenerHelper - replacement for LambdaMultiplePropertyChangeListenerHandler .
Please take a look at https://github.com/openjdk/jfx/pull/908
-------------
PR: https://git.openjdk.org/jfx/pull/906
