On Wed, 10 Feb 2021 15:24:07 GMT, Jeanette Winzenburg <faste...@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

Reply via email to