RFR: 8252935: Add treeShowing listener only when needed [v4]

John Hendrikx jhendrikx at openjdk.java.net
Wed Feb 10 21:56:02 UTC 2021


On Wed, 10 Feb 2021 15:24:07 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

>> thanks for the info :)
>
>> I could extract the methods instead if you donot like me passing in `null` for the `ObservableValue`, it would then look like:
>> 
>> ```
>>     sceneChanged(null, node.getScene());  // register chain
>> 
>>     sceneChanged(node.getScene(), null);  // unregister chain
>> ```
>> 
> 
> That's a good pattern, IMO (biased, using it in my own code)
> 
>> 
>> Also I should mention that this is not Skin code in case it was missed.
> 
> No, it was not missed :)  Aside: actually, we cannot use the pattern above in skin code because we are using LambdaMultipleListenerHandler to un/register for change notification. The consumer that's registered carries only the observable, consequently its old value is not available at notification time. Which forces keeping the old value stored in the skin, if interested in changes from a "path" property.

I saw the failures in github actions, but figured it was an intermittent failure as I couldn't see an obvious test failure.  Somehow the local build didn't run the test again, so I missed it completely.  I've pushed a fix.

I'm happy this pattern is more to your liking, it does look a bit cleaner and saves some duplication.

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

PR: https://git.openjdk.java.net/jfx/pull/185


More information about the openjfx-dev mailing list