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