On Fri, 14 Oct 2022 23:01:48 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> I agree with your first point, but I think limiting any changes to `Node`
>> because it would add another field is not a good enough argument. It would
>> depend on its merits.
>>
>> The second point, there is much more to the `shownProperty` then you might
>> think at first glance. In combination with `when`, it offers a standard way
>> to manage life cycles for controls. It is a very natural fit to tie the
>> lifecycle of bindings and listeners to whether a control is actually
>> currently shown. You may think that a Listener helper class would be just
>> as flexible, but it isn't. The helper is not available everywhere (unless
>> you propose to integrate it into Node), which means when constructing
>> complicated UI's which may have smaller groups of controls that are created
>> by other classes, methods or factories, that you may have to pass along an
>> instance of your helper to all these to make sure they register their
>> bindings and listeners with it (or perhaps to link multiple Listener helper
>> classes together in some kind of hierarchy, that matches the control
>> groupings that might have separate life cycles).
>>
>> The `shownProperty` does not have this problem. Since you can tie the
>> lifecycle to your own small piece of the UI (ie, a factory that creates a
>> `BorderPane` with several controls, can tie all these to the lifecycle of
>> the `BorderPane`). When this `BorderPane` is just a piece of UI used as
>> part of a larger whole, its lifecycle still works as expected -- if it is
>> not shown, its listeners disappear. If the whole window is destroyed, then
>> the same happens. If however the Pane next to it is destroyed, only their
>> listeners disappear. With a helper system, you'd need to coordinate this
>> somehow, perhaps having that `BorderPane` return its helper, so the large UI
>> can tie it to its helper, so when the main helper is called to unsubscribe,
>> that all its child helpers are also unsubscribed...
>>
>> Another advantage is that a listener does not have to start working right
>> away. When setting up a new pane of controls, many subscriptions may need
>> to be made. When using `when(control::shownProperty)` these listeners don't
>> immediately become active (potentially while still constructing and
>> initializing your control). Only when the control is fully constructed, and
>> added to an active Scene and Window do all the listeners become active.
>> This can be important when listening to external properties that change
>> often and may even change during construction.
>>
>> In ReactFX it the `shownProperty` was deemed important enough to go one step
>> further and even create a separate method on what would be `ObservableValue`
>> called `conditionOnShowing`.
>>
>> In short, `when` and the `shownProperty` is basically used every time you
>> create a binding or add a listener between two properties that have a
>> different lifecycle, like between a model and a control. This completely
>> removes the responsibility of having to track these bindings or listeners
>> separately; it becomes automatic. Subscribing all at once when it becomes
>> first shown becomes automatic (and you immediately get the first value),
>> unsubscribing when it goes invisible is automatic, and resubscribing if a
>> control becomes shown again is completely transparent.
>>
>> In my projects, I've completely eliminated memory leaks without a single
>> call to `removeListener` or `unbind` anywhere.
>> There are no explicit weak listeners (and not just because they're
>> incredibly cumbersome to use), nor is there anywhere where subscriptions or
>> bindings are being tracked (aside from the `when` construct). I'm very
>> picky about memory leaks, so I have a Scene scanner, that maintains weak
>> references to all Nodes at all times, and warns me if any are not getting
>> GC'd after not being part of a Scene for a while. In 99% of the cases, I
>> forgot a `when`, where in plain JavaFX you'd be forced to use a weak
>> listener.
>>
>> The simple rule to follow is, whenever you bind or listen to or on a
>> property, if they have different lifecycles, then use a `when`, otherwise
>> bind or listen directly.
>>
>> Some examples of how it is used:
>>
>> Below is a very common pattern when binding any property that has a
>> different lifecycle than its target, this is one of many:
>>
>> longLivedModel.selectedItem
>> .when(this::shownProperty) // this = BorderPane, and the code is
>> part of its constructor)
>> .addListener((obs, old, current) -> {
>> // code that starts a transition from the previous item to the
>> next item
>> }
>> );
>>
>> Below some binding examples where several properties are bound to long lived
>> versions:
>>
>> ObservableValue<Boolean> shown = pane.shownProperty();
>>
>> pane.model.work.bind(presentation.root.when(shown));
>>
>> pane.model.missingFraction.bind(presentation.missingFraction.when(shown));
>>
>> pane.model.watchedFraction.bind(presentation.watchedFraction.when(shown));
>>
>> Below a "bidirectional binding" for a `Slider` control, where one direction
>> is long lived. The binding vanishes on its own when the UI that is using it
>> is closed -- this is part of a UI generator, so this code has no idea where
>> the UI might end up being used or for how long, but it can still manage the
>> lifecycle easily:
>>
>>
>> longLivedValue.when(slider::shownProperty).map(toNumber).subscribe(slider.valueProperty()::setValue);
>>
>> slider.valueProperty().map(fromNumber).subscribe(longLivedValue::setValue);
>>
>> Another example to make sure a `Timeline` is stopped/started automatically
>> based on visibility (Timeline's will keep references around if not stopped):
>>
>> center.shownProperty() // center here is a Pane
>> .addListener((obs, old, visible) -> {
>> if(visible) timeline.playFromStart();
>> else timeline.stop();
>> });
>>
>> So, I think it certainly has some merit to include it :)
>>
>>> And another thing - Window and Dialog are not Nodes, but they do have
>>> showing property.
>>
>> I'm unsure what you are saying here. I don't think we need to tie anything
>> directly to a Window or Dialog, only to controls they might contain. Also
>> the `shownProperty` is not useful for properties not associated with
>> controls; depending on what the properties are used for people may have to
>> create their own `active` property or something else that identifies the
>> lifecycle of those properties. For control properties though, their
>> lifecycle is quite clear and I think it makes great sense to tie to whether
>> they're actually visible or not.
>
> What I meant is you can add it in Node.getProperties() and not to add a
> pointer to every Node everywhere. If it's used, the actual properties will
> be created in a few containers.
private static final Object KEY = new Object();
public final ReadOnlyBooleanProperty shownProperty() {
Object x = getProperties().get(KEY);
if (x instanceof ReadOnlyBooleanProperty p) {
return p;
}
ReadOnlyBooleanProperty p = ... // create
getProperties().put(KEY, p);
return p;
}
-------------
PR: https://git.openjdk.org/jfx/pull/830
