On Mon, 7 Feb 2022 16:17:32 GMT, Nir Lisker <[email protected]> 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
