On Sun, 4 Sep 2022 19:49:42 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> This PR adds a new (lazy*) property on `Node` which provides a boolean which 
>> indicates whether or not the `Node` is currently part of a `Scene`, which in 
>> turn is part of a currently showing `Window`.
>> 
>> It also adds a new fluent binding method on `ObservableValue` dubbed `when` 
>> (open for discussion, originally I had `conditionOn` here).
>> 
>> Both of these together means it becomes much easier to break strong 
>> references that prevent garbage collection between a long lived property and 
>> one that should be shorter lived. A good example is when a `Label` is bound 
>> to a long lived property:
>> 
>>      
>> label.textProperty().bind(longLivedProperty.when(label::isShowingProperty));
>> 
>> The above basically ties the life cycle of the label to the long lived 
>> property **only** when the label is currently showing.  When it is not 
>> showing, the label can be eligible for GC as the listener on 
>> `longLivedProperty` is removed when the condition provided by 
>> `label::isShowingProperty` is `false`.  A big advantage is that these 
>> listeners stop observing the long lived property **immediately** when the 
>> label is no longer showing, in contrast to weak bindings which may keep 
>> observing the long lived property (and updating the label, and triggering 
>> its listeners in turn) until the next GC comes along.
>> 
>> The issue in JBS also describes making the `Subscription` API public, but I 
>> think that might best be a separate PR.
>> 
>> Note that this PR contains a bugfix in `ObjectBinding` for which there is 
>> another open PR: https://github.com/openjdk/jfx/pull/829 -- this is because 
>> the tests for the newly added method would fail otherwise; once it has been 
>> integrated to jfx19 and then to master, I can take the fix out.
>> 
>> (*) Lazy means here that the property won't be creating any listeners unless 
>> observed itself, to avoid problems creating too many listeners on 
>> Scene/Window.
>
> John Hendrikx has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Small wording fixes

The docs of `when` look good now (I left one minor comment). I actually didn't 
think about the name of the method, compared to ReactFX's `conditionOn` for 
example, but this is good enough until others review.

I haven't looked at the addition to `Node` closely yet. I will look at the 
implementation and tests of the method itself first because `Node` is a use 
case.

modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java line 
296:

> 294:      * 
> label.textProperty().bind(longLivedProperty.when(label::isShownProperty));
> 295:      * }</pre>
> 296:      * @param condition a boolean {@code ObservableValue}, cannot be 
> {@code null}

New line before `@param`.

-------------

Changes requested by nlisker (Reviewer).

PR: https://git.openjdk.org/jfx/pull/830

Reply via email to