The chapter with leaking skins is almost completed: memory leaks are plugged, the testing framework is in place, switching is fully supported.
The only open ticket under umbrella JDK-8241364<https://bugs.openjdk.org/browse/JDK-8241364> ☂ Cleanup skin implementations to allow switching is due to InputMap https://bugs.openjdk.org/browse/JDK-8245303 , which can be extracted into a separate issue (and which will be solved by design if the InputMap v2 proposal is accepted). As far as the removal goes, we do have JDK-8296076<https://bugs.openjdk.org/browse/JDK-8296076> Remove internal class LambdaMultiplePropertyChangeListenerHandler which is waiting its turn. -andy From: openjfx-dev <[email protected]> on behalf of Marius Hanl <[email protected]> Date: Tuesday, March 12, 2024 at 11:27 To: [email protected] <[email protected]>, John Hendrikx <[email protected]> Subject: Aw: Re: Skin ListenerHelper Thanks for providing the examples with the Subscription API. Note though that I rather want to talk about that we have 2 different components taking care of the listeners and both are mixed up. ListenerHelper is totally fine for me, but that should be the only one. It would be nice to complete the chapter of leaking skins completely before tackling the next big changes. My opinion though. -- Marius Am 12.03.24, 09:40 schrieb John Hendrikx <[email protected]>: With subscriptions these kinds of helpers are really not needed anymore. Just do: Subscription combined = Subscription.combine( property1.subscribe( ... ), property2.subscribe( ... ) ); And clean it up with `combined.unsubscribe` in `dispose`. For non-property clean-ups, you can add additional cleanup like this: Subscription combined = subscription.and(() -> { extra clean-up code }); Alternatively, you can use `when(skinActive)` on most of your observers, and set `skinActive` to `false` in dispose. SkinBase was a mistake IMHO. Helper methods that require state are better placed in an instanced utility, not in a base class. What does it mean that a "TableViewSkin" is-a "SkinBase". Using inheritance to get access to such helpers (which need a little bit of state) is really a classic mistake and misuse of inheritance. ListenerHelper is much better in that regard, much more generally usable (since it uses composition), and stores its state in its own instance. --John On 12/03/2024 00:19, Marius Hanl wrote: Since I see multiple discussions about the behaviour API and the RichTextControl, I want to ask another, still important question: What is about the Skin cleanup ListenerHelper? We now have 2 implementations of such utility - the LambdaMultiplePropertyChangeListenerHandler and the ListenerHelper I would really see that this topic is closed and cleaned up, with all TODOs that emerged from this than 2-3 new other open implemention changes on different topics. The class is also not public, so extending from something else than SkinBase makes it hard to write your own skin as there is no proper help to register and dispose your listener. -- Marius
