On Tue, 25 Oct 2022 20:54:55 GMT, Nir Lisker <[email protected]> wrote:
>> John Hendrikx has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains ten commits:
>>
>> - Merge remote-tracking branch 'origin/master' into
>> feature/conditional-bindings
>>
>> # Conflicts:
>> #
>> modules/javafx.base/src/test/java/test/javafx/beans/value/LazyObjectBindingTest.java
>> - Fix review comments
>> - Add missing new line
>> - Small wording fixes
>> - Update javadoc of "when" with better phrasing
>> - Rename showing property to shown as it is already used in subclasses
>> - Add conditional bindings to ObservableValue
>> - Change explanatory comment to block comment
>> - Fix bug where ObjectBinding returns null when revalidated immediately
>>
>> Introduced with the fluent bindings PR
>
> modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 1417:
>
>> 1415: * to be {@code true}.
>> 1416: *
>> 1417: * @defaultValue false
>
> I don't think a default value is relevant. It can't be set and the node is
> not responsible for its value. It's true, though, that by default, since the
> node does not belong to a window, it's `false`, but I don't know how helpful
> it is.
You're right, this is a copy/paste error. Makes no sense for a read only
property. I will remove it.
> modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 1426:
>
>> 1424: if (s == null) return false;
>> 1425: Window w = s.getWindow();
>> 1426: return w != null && w.isShowing();
>
> Are you avoiding calling `get` on the property to avoid its initialization?
Yeah, I actually copied this pattern from an older method that has since been
removed (`isWindowShowing`) in an effort to avoid initializing the property if
all you're doing is calling its getter. I personally don't mind either way,
but as it seems `Node` goes through every effort to delay initialization, I
followed that pattern. It does duplicate the logic of the property which uses
`flatMap`s to achieve the same.
-------------
PR: https://git.openjdk.org/jfx/pull/830
