[jfx15] RFR: 8228570: Add various documentation clarifications [v2]

[Kevin Rushforth]

On Tue, 28 Jul 2020 18:42:46 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>> Adds clarifications to the documentation in various places. Some notes:
>> 
>> * Point 6 should probably be deferred until it is verified that the tutorials are correct enough, seeing as they were
>>   updated to Java 8 only.
>> * Point 8 has been deferred until all the animation bugs have been resolevd.
>> * Point 5: I wrote new documentation about the `extractor` for the `observableArrayList(Callback<E, Observable[]>
>>   extractor)` method. Later I found that `observableList(List<E> list, Callback<E, Observable[]> extractor)` already
>>   talks about it (I updated it too). I'm not sure which of them we want to keep, or maybe merge them.
>> * Point 1: I think that it's necessary to mention the internal implementation behavior even if it requires a caveat that
>>   this is only the current behavior and may change in the future. What constitutes a "change" is extremely important and
>>   there is no way for the user to know it. I've tripped on this hard when using ReactFX which uses object equality
>>   instead, so when the JavaFX observables are wrapped by ReactFX observables, the behavior changes silently.
>> I think that in the future we will want to let the user define what a change is (for example, by creating an
>> overridable method with the current behavior as the default, or using object equality and letting the user override
>> that, although that's more risky). Even a `HashMap` that uses object equality has the sister implementation
>> `IndentityHashMap` to deal with this ambiguity.
>
> Nir Lisker has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Corrected javadoc generation errors

I left a few formatting comments and a substantive one on the clarification of the transform order, which I recommend
to defer to 16.

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

> 47:  * Current implementing classes in JavaFX check for a change using reference
> 48:  * equality (and not object equality, {@code Object#equals(Object)}) of the value. An
> 49:  * invalidation event is generated if the current value is not valid anymore.

I think this is fine given the way it is stated. It might be good to move it to an `@implSpec` section, along with the
previous paragraph regarding lazy evaluation, but that would require a CSR and can be considered for a future release.

modules/javafx.base/src/main/java/javafx/collections/FXCollections.java line 114:

> 113:      * Observable objects returned by the extractor (applied to each list element) are listened for changes
> 114:      * and transformed into {@link ListChangeListener.Change#wasUpdated() "update"} changes of a {@code
> ListChangeListener}. 115:      *

I recommend `@linkplain` here since "update" is not the name of a method or property.

modules/javafx.controls/src/main/java/javafx/scene/control/Labeled.java line 804:

> 803:      *
> 804:      * @defaultValue {@code false}; {@code true} for some Controls.
> 805:      */

Either `controls` (lower case) or `{@code Control}s` seems better.

modules/javafx.graphics/src/main/java/javafx/animation/Timeline.java line 146:

> 145:     /**
> 146:      * Creates a {@code Timeline} with no key frames and a {@link Animation#getTargetFramerate() target framerate}.
> 147:      *

`@linkplain`

modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 868:

> 867:      * clearing the map or overriding its values. These entries are not removed automatically if the node is
> removed from the layout 868:      * manager, so unused entries can exist throughout the life of the node.
> 869:      *

If we were in the habit of using `@apiNote` then that's probably where the note about layout managers using the node
properties would go. Since we aren't, this seems fine.

modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 328:

> 327:  * {@link #translateXProperty translateX}, {@link #translateYProperty translateY} and {@link #translateZProperty
> translateZ} 328:  * properties.
> 329:  *

See my comment on the transform property. This section seems like the right place to expand the documentation to
specify the order of matrix multiplication and the order in which they affect the node's geometry.

modules/javafx.controls/src/main/java/javafx/scene/control/Labeled.java line 269:

> 268:      * @since JavaFX 2.2
> 269:      * @defaultValue {@code "..."}
> 270:      */

This should go before the `@since` (which is usually the last tag)

modules/javafx.controls/src/main/java/javafx/scene/control/Labeled.java line 613:

> 612:      * @since JavaFX 8.0
> 613:      * @defaultValue 0
> 614:      */

Before `@since`

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

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