[jfx20] RFR: 8290863: Update the documentation of Virtualized controls to include the best practice of not using Nodes directly in the item list [v3]

[Kevin Rushforth]

On Wed, 18 Jan 2023 09:25:08 GMT, Ajit Ghaisas <aghaisas at openjdk.org> wrote:

>> This PR adds a warning about inserting Nodes directly into the virtualized containers such as ListView, TreeView, TableView and TreeTableView. It also adds code snippets showing the recommended pattern of using a custom cell factory for each of the virtualized control.
>
> Ajit Ghaisas has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove extra spaces

Overall it looks great. I left a few comments on the wording.

I presume all of the newly added examples compile?

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

modules/javafx.controls/src/main/java/javafx/scene/control/ListView.java line 161:

> 159:  * provide a custom {@link #cellFactoryProperty() cell factory} to create the nodes for a
> 160:  * given cell and update them on demand using the data stored in the item for that cell.</li>
> 161:  * <li>Avoid creating new {@link Node}s in custom {@link #cellFactoryProperty() cell factory} {@code updateItem} method.</li>

I think it would read better to move this bullet above the previous so that the two "avoid"s are next to each other.

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"

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

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

PR: https://git.openjdk.org/jfx/pull/995