[jfx18] RFR: 8271085: TabPane: Redundant API docs

[Ajit Ghaisas]

On Mon, 7 Feb 2022 16:17:32 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>> This is a Javadoc cleanup and correction fix for the TabPane as described in the JBS.
>> 
>> Changes done for all the Properties of the TabPane -
>> - Moved the property description to be over the property field.
>> - Removed the unnecessary docs on property setter/getter and Property method.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/TabPane.java line 174:
> 
>> 172:     /**
>> 173:      * <p>Sets the model used for tab selection.  By changing the model you can alter
>> 174:      * how the tabs are selected and which tabs are first or last.</p>
> 
> This description is phrased like it's a setter. Probably
> 
> The selection model used for selecting tabs. Changing the model alters
> how the tabs are selected and which tabs are first or last.
> 
> (there's no need to the `<p>` either)

Fixed.

> modules/javafx.controls/src/main/java/javafx/scene/control/TabPane.java line 188:
> 
>> 186:      * the TabPane will immediately update the location of the tabs to reflect
>> 187:      * this.</p>
>> 188:      * The default position for the tabs is {@code Side.Top}.
> 
> I would rephrase a bit:
> 
> The position of the tabs in the {@code TabPane}. Changes to the value of this property
> immediately update the location of the tabs.
> 
> @defaultValue {@code Side.Top}
> 
> The 2nd sentence seems redundant anyway and I suggest to remove it. Unless otherwise specified, all value changes are applied immediately (only `Animation` properties come to mind that specify that the animation has to be stopped for the changes to take effect).

I like the rephrased version. I prefer keeping the 2nd sentence in this case as it indicates a non-trivial visual update.

> modules/javafx.controls/src/main/java/javafx/scene/control/TabPane.java line 244:
> 
>> 242:      * <p>Refer to the {@link TabClosingPolicy} enumeration for further details.</p>
>> 243:      *
>> 244:      * The default closing policy is TabClosingPolicy.SELECTED_TAB
> 
> * The default value should be in an `@defaultValue` annotation.
> * Missing `{@code }`s
> * The line "Refer to the {@link TabClosingPolicy}" is redundant I think since it's linked in the signature/definition, and if we want to keep it then an `@see` might be preferable.
> * "end-user's" (missing apostrophe)

Fixed points 1, 2 and 4.
Regarding point 3 - 
I have removed the description regarding possible options and have kept only the line-
`Refer to the {@link TabClosingPolicy} enumeration for further details.`
Mentioning all the options here seems to be a duplication of Javadoc for enumeration TabClosingPolicy.

> modules/javafx.controls/src/main/java/javafx/scene/control/TabPane.java line 270:
> 
>> 268:      * the graphic isn't rotated, resulting in it always appearing upright. If
>> 269:      * rotateGraphic is set to {@code true}, the graphic will rotate such that it
>> 270:      * rotates with the tab text.</p>
> 
> * Missing `@code`s
> * I would rephrase the 2nd paragraph to be simpler:
>   ```
>   If the value is {@code false}, the graphic isn't rotated, resulting in it always appearing upright.
>   If it is {@code true}, the graphic is rotate with the tab text.
>   ```
> * `@defaultValue {@code false}`

Fixed.

> modules/javafx.controls/src/main/java/javafx/scene/control/TabPane.java line 295:
> 
>> 293:      *
>> 294:      * This value can also be set via CSS using {@code -fx-tab-min-width}.
>> 295:      * </p>
> 
> I would write
> 
> The minimum width of a tab in the TabPane. This can be used to limit
> the length of text in tabs to prevent truncation.  Setting the same minimum
> and maximum widths will fix the width of the tab.
> 
> It makes it clear that it applies to each tab individually (and not the total width of the tabs). The same applies for the other min/max height/width properties.
> 
> The sentence "By default the min equals to the max." seems wrong. The `DEFAULT_TAB_MIN_WIDTH` constant is set to 0. It should also be in a `@defaultValue`.
> 
> The CSS mention is good, bit I never saw it mentioned for properties before. Makes me wonder if we should add the CSS property as part of a property's description in general via the automated javadoc tool.

Good suggestion. Fixed.

Regarding CSS mention for properties, I am not sure whether that can be automated. If there is a way to achieve it, I can file a separate bug to address it.

> modules/javafx.controls/src/main/java/javafx/scene/control/TabPane.java line 337:
> 
>> 335:      *
>> 336:      * This value can also be set via CSS using {@code -fx-tab-max-width}.
>> 337:      * </p>
> 
> * "By default the min equals to the max." The default is `Double.MAX_VALUE`.
> * Comma before "the text will be truncated".

Fixed.

> modules/javafx.controls/src/main/java/javafx/scene/control/TabPane.java line 378:
> 
>> 376:      *
>> 377:      * This value can also be set via CSS using {@code -fx-tab-min-height}.
>> 378:      * </p>
> 
> Same comments as for the width properties.

Fixed.

> modules/javafx.controls/src/main/java/javafx/scene/control/TabPane.java line 419:
> 
>> 417:      *
>> 418:      * This value can also be set via CSS using {@code -fx-tab-max-height}.
>> 419:      * </p>
> 
> Same comments as for the width properties.

Fixed.

> modules/javafx.controls/src/main/java/javafx/scene/control/TabPane.java line 779:
> 
>> 777:      * The drag policy for the tabs. The policy can be changed dynamically.
>> 778:      *
>> 779:      * @defaultValue TabDragPolicy.FIXED
> 
> I think that the 2nd sentence is redundant again. I would add an explanation like "Specifies if tabs can be reordered or not" to elaborate on what a "drag policy" is.
> 
> Also, `{@code TabDragPolicy.FIXED}`.

Fixed.

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

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