On Wed, 18 Jan 2023 23:20:10 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Ajit Ghaisas has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove extra spaces
>
> modules/javafx.controls/src/main/java/javafx/scene/control/ComboBox.java line
> 126:
>
>> 124: * necessary to specify a custom {@link StringConverter}.
>> 125: *
>> 126: * <h2>Warning: Nodes should not be inserted directly into the ComboBox
>> items list</h2>
>
> Can you also add the bulleted list of "Important points to note" to
> `ComboBox`?
Added the bulleted list.
> I think it would read better to move this bullet above the previous so that
> the two "avoid"s are next to each other.
The current flow of 3 bullets seems natural to me.
The first point says - what to avoid?
The second point says - what is our recommendation.
The third point says - what to avoid in our recommendation.
> Also, that should either be "a custom cell factory updateItem method" (adding
> the missing article) or maybe "the updateItem method of a custom cell factory"
Fixed.
> modules/javafx.controls/src/main/java/javafx/scene/control/ListView.java line
> 191:
>
>> 189: * <p> This example has an anonymous custom {@code ListCell} class in
>> the custom cell factory.
>> 190: * Note that the {@code Rectangle} ({@code Node}) object needs to be
>> created in the custom {@code ListCell} class
>> 191: * or in its constructor and updated/used in its {@code updateItem}
>> method.
>
> I might just say "created in the constructor of the custom ListCell" and
> leave it at that. Saying "in the ... class or ... constructor" might be
> confusing, since the node is an instance field. The example uses an instance
> initialization block, which you could mention if you want to be more precise
> than just saying "in the constructor".
Reworded the description to include "instance initialisation block"
-------------
PR: https://git.openjdk.org/jfx/pull/995
