RFR: 8271091: Missing API docs in UI controls classes [v2]

[Kevin Rushforth]

On Thu, 21 Oct 2021 08:58:37 GMT, Ajit Ghaisas <aghaisas at openjdk.org> wrote:

>> This PR fixes javadoc warnings in javafx.controls and javafx.web modules.
>> Note : 
>> - The javadoc needs to be generated with the JDK 18 EA build.
>> - There are still 20 javadoc warnings remaining in javafx.controls module and 3 warnings remaining in javafx.web module. The root cause is different and they will be addressed under [JDK-8270996](https://bugs.openjdk.java.net/browse/JDK-8270996)
>
> Ajit Ghaisas has updated the pull request incrementally with one additional commit since the last revision:
> 
>   javadoc minor corrections

See comments below.

modules/javafx.controls/src/main/java/javafx/scene/control/DatePicker.java line 470:

> 468:      * {@code CssMetaData} of its superclasses.
> 469:      * @return the {@code CssMetaData}
> 470:      * @since JavaFX 8.0

The class already has an `@since JavaFX 8.0` tag so this is unnecessary. It's also unrelated to this fix (and we would need a CSR for any changes to `@since` tags).

modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java line 160:

> 158:     /**
> 159:      * Focuses the item at the given index.
> 160:      * @param index the index of the item that needs to be focused

Minor: I recommend removing "that needs" from this `@param`.

modules/javafx.controls/src/main/java/javafx/scene/control/PopupControl.java line 201:

> 199:      * this specific {@code PopupControl}.
> 200:      * @return the {@code StringProperty} representing the CSS style
> 201:      */

I recommend taking the information from the setter and copying it here. The docs can/should then be removed from both the setter and the getter. The first sentence should describe the property so does not need to start with "Gets the ...". You can add the information about `null` being turned into the empty string as a sentence in the Description. The `@defaultValue empty string` means that the information in the `@return` of the getter is unnecessary.

modules/javafx.controls/src/main/java/javafx/scene/control/SortEvent.java line 47:

> 45:     /**
> 46:      * Gets the default singleton {@code SortEvent}.
> 47:      * @param <C> the type of control

Can you also add this `@param` tag to the class description? I'm a little surprised that javadoc doesn't flag it as missing.

modules/javafx.controls/src/main/java/javafx/scene/control/SortEvent.java line 59:

> 57:     /**
> 58:      * Constructs a new {@code Event} with the specified event source, target
> 59:      * and type. If the source or target is set to {@code null}, it is replaced

That should be "a new `{@code SortEvent}` ...". Also, since there is no type parameter, you should remove "and type" (and replace the comma after source with "and").

modules/javafx.controls/src/main/java/javafx/scene/control/SortEvent.java line 63:

> 61:      *
> 62:      * @param source the event source which sent the event
> 63:      * @param target the target of the scroll to operation

That should be "the target of the event".

modules/javafx.controls/src/main/java/javafx/scene/control/TabPane.java line 405:

> 403:     }
> 404: 
> 405:     public final DoubleProperty tabMaxWidthProperty() {

The best practice for properties is to put the description on exactly one of the `xxx` private field or the `xxxProperty` method, and not on the setter or getter (unless there is a specific need to document something special in the setter or getter). There is a separate JBS issue, [JDK-8271085](https://bugs.openjdk.java.net/browse/JDK-8271085), tracking the fix for the `maxWidth` and `maxHeight` properties.

You can either revert this change, in which case we'll need a new PR for that issue (it can be done later), or else modify this change to match the best practice as noted in JDK-8271085 and add that issue to this PR. The latter will require addressing the other methods in this file as noted in that JBS bug.

modules/javafx.controls/src/main/java/javafx/scene/control/TabPane.java line 501:

> 499:     }
> 500: 
> 501:     public final DoubleProperty tabMaxHeightProperty() {

Same comment as above. This needs to be reverted or modified.

modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableColumn.java line 625:

> 623:      * Gets the {@code CssMetaData} associated with this class, which may include the
> 624:      * {@code CssMetaData} of its superclasses.
> 625:      * @return empty list is returned as of now

Minor: I recommend adding a second sentence to the Description to the effect that it is currently an empty list, and then have the return tag be simply `@return an empty list`

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TextInputControlSkin.java line 118:

> 116:     /** Line unit */      LINE,
> 117:     /** Paragraph unit */ PARAGRAPH,
> 118:     /** Page unit */      PAGE };

I would prefer that the javadoc tag and the enum be on separate lines.


    /** Character unit */
    CHARACTER,
    /** Word unit */
    WORD,
    ...

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TextInputControlSkin.java line 602:

> 600: 
> 601:     /**
> 602:      * Get the path elements describing the shape of the underline for the given range.

Gets

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TextInputControlSkin.java line 609:

> 607:     protected abstract PathElement[] getUnderlineShape(int start, int end);
> 608: 
> 609:     /** Get the path elements describing the bounding rectangles for the given range of text.

Gets

modules/javafx.web/src/main/java/javafx/scene/web/HTMLEditorSkin.java line 1168:

> 1166:      */
> 1167:     public enum Command {
> 1168:         /** Cut command.*/    CUT("cut"),

Same comment as in `TextInputControlSkin`.

modules/javafx.web/src/main/java/javafx/scene/web/HTMLEditorSkin.java line 1212:

> 1210: 
> 1211:         /**
> 1212:          * Get the name of this command.

Gets

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

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