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

Nir Lisker nlisker at openjdk.java.net
Mon Feb 7 18:07:15 UTC 2022


On Mon, 7 Feb 2022 10:53:00 GMT, Ajit Ghaisas <aghaisas 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.

Moving the test to the property field and removing it from the methods looks good. I added some suggestions to touch up the docs a bit and correct some mistakes.

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)

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).

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)

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}`

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.

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".

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.

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.

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}`.

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

Changes requested by nlisker (Reviewer).

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


More information about the openjfx-dev mailing list