Summary

Add a warning about inserting Nodes directly into the virtualized controls such as ListView, TreeView, TableView, TreeTableView and ComboBox. Also add code snippets showing the recommended pattern of using a custom cell factory for each of the virtualized controls.

Problem

We have come across many cases of improper usage of Node instances being put directly into virtualized controls such as ListView, TreeView, TableView, TreeTableView and ComboBox. Inserting UI element such as a Node into the data model of an application does not scale well and result in UI or memory issues.

Solution

Although inserting Node instances directly into virtualized controls is not blocked, it is not a recommended pattern. A recommended approach is to provide a custom cell factory to create required Nodes dynamically using data stored in the data model of the application.

A lot of application developers know this separation of Data Model and UI elements. This proposed change adds an explicit warning for all virtualized controls (ListView, TreeView, TableView, TreeTableView and ComboBox). It also adds code snippets showing the recommended pattern of using a custom cell factory for each of these controls.

Specification

diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/ComboBox.java b/modules/javafx.controls/src/main/java/javafx/scene/control/ComboBox.java
index c571bdd45c5..23dbbd6efe1 100644
--- a/modules/javafx.controls/src/main/java/javafx/scene/control/ComboBox.java
+++ b/modules/javafx.controls/src/main/java/javafx/scene/control/ComboBox.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -123,10 +123,11 @@
  * a different type is specified and the ComboBox is to be editable, it is
  * necessary to specify a custom {@link StringConverter}.
  *
- * <h2>A warning about inserting Nodes into the ComboBox items list</h2>
- * ComboBox allows for the items list to contain elements of any type, including
+ * <h2>Warning: Nodes should not be inserted directly into the ComboBox items list</h2>
+ * {@code ComboBox} allows for the items list to contain elements of any type, including
  * {@link Node} instances. Putting nodes into
- * the items list is <strong>strongly not recommended</strong>. This is because
+ * the items list is <strong>strongly discouraged</strong>, as it can
+ * lead to unexpected results. This is because
  * the default {@link #cellFactoryProperty() cell factory} simply inserts Node
  * items directly into the cell, including in the ComboBox 'button' area too.
  * Because the scenegraph only allows for Nodes to be in one place at a time,
@@ -134,18 +135,16 @@
  * list, and becomes visible in the button area. When selection changes the
  * previously selected item returns to the list and the new selection is removed.
  *
- * <p>The recommended approach, rather than inserting Node instances into the
- * items list, is to put the relevant information into the ComboBox, and then
- * provide a custom {@link #cellFactoryProperty() cell factory}. For example,
- * rather than use the following code:
- *
- * <pre> {@code ComboBox<Rectangle> cmb = new ComboBox<>();
- * cmb.getItems().addAll(
- *     new Rectangle(10, 10, Color.RED),
- *     new Rectangle(10, 10, Color.GREEN),
- *     new Rectangle(10, 10, Color.BLUE));}}</pre>
- *
- * <p>You should do the following:</p>
+ *<p>Important points to note:
+ * <ul>
+ * <li>Avoid inserting {@code Node} instances directly into the {@code ComboBox} items list or its data model.</li>
+ * <li>The recommended approach is to put the relevant information into the items list, and
+ * provide a custom {@link #cellFactoryProperty() cell factory} to create the nodes for a
+ * given cell and update them on demand using the data stored in the item for that cell.</li>
+ * <li>Avoid creating new {@code Node}s in the {@code updateItem} method of
+ * a custom {@link #cellFactoryProperty() cell factory}.</li>
+ * </ul>
+ * <p>The following minimal example shows how to create a custom cell factory for {@code ComboBox} containing {@code Node}s:
  *
  * <pre><code> ComboBox&lt;Color&gt; cmb = new ComboBox&lt;&gt;();
  * cmb.getItems().addAll(
@@ -173,6 +172,9 @@
  *         }
  *     };
  * });</code></pre>
+ * <p> This example has an anonymous custom {@code ListCell} class in the custom cell factory.
+ * Note that the {@code Rectangle} ({@code Node}) object needs to be created in the instance initialization block
+ * or the constructor of the custom {@code ListCell} class and updated/used in its {@code updateItem} method.
  *
  * <img src="doc-files/ComboBox.png" alt="Image of the ComboBox control">
  *
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/ListView.java b/modules/javafx.controls/src/main/java/javafx/scene/control/ListView.java
index f232606d862..fd71162aa7e 100644
--- a/modules/javafx.controls/src/main/java/javafx/scene/control/ListView.java
+++ b/modules/javafx.controls/src/main/java/javafx/scene/control/ListView.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -145,7 +145,50 @@
  * default {@link #cellFactoryProperty() cell factory}. A cell factory is used to
  * generate {@link ListCell} instances, which are used to represent an item in the
  * ListView. See the {@link Cell} class documentation for a more complete
- * description of how to write custom Cells.
+ * description of how to write custom Cells.</p>
+ *
+ * <h3>Warning: Nodes should not be inserted directly into the items list</h3>
+ * {@code ListView} allows for the items list to contain elements of any type, including
+ * {@link Node} instances. Putting nodes into
+ * the items list is <strong>strongly discouraged</strong>, as it can
+ * lead to unexpected results.
+ * <p>Important points to note:
+ * <ul>
+ * <li>Avoid inserting {@code Node} instances directly into the items list or its data model.</li>
+ * <li>The recommended approach is to put the relevant information into the items list, and
+ * provide a custom {@link #cellFactoryProperty() cell factory} to create the nodes for a
+ * given cell and update them on demand using the data stored in the item for that cell.</li>
+ * <li>Avoid creating new {@code Node}s in the {@code updateItem} method of a custom {@link #cellFactoryProperty() cell factory}.</li>
+ * </ul>
+ * <p>The following minimal example shows how to create a custom cell factory for {@code ListView} containing {@code Node}s:
+ *
+ * <pre>{@code   ListView<Color> lv = new ListView<>();
+ *  lv.getItems().addAll(Color.RED, Color.GREEN, Color.BLUE);
+ *
+ *  lv.setCellFactory(p -> {
+ *    return new ListCell<>() {
+ *        private final Rectangle rectangle;
+ *        {
+ *            setContentDisplay(ContentDisplay.GRAPHIC_ONLY);
+ *            rectangle = new Rectangle(10, 10);
+ *        }
+ *
+ *        @Override
+ *        protected void updateItem(Color item, boolean empty) {
+ *            super.updateItem(item, empty);
+ *
+ *            if (item == null || empty) {
+ *                setGraphic(null);
+ *            } else {
+ *                rectangle.setFill(item);
+ *                setGraphic(rectangle);
+ *            }
+ *         }
+ *     };
+ * });}</pre>
+ * <p> This example has an anonymous custom {@code ListCell} class in the custom cell factory.
+ * Note that the {@code Rectangle} ({@code Node}) object needs to be created in the instance initialization block
+ * or the constructor of the custom {@code ListCell} class and updated/used in its {@code updateItem} method.
  *
  * <h2>Editing</h2>
  * <p>This control supports inline editing of values, and this section attempts to
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java b/modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java
index 146e74fbccd..851d818c85b 100644
--- a/modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java
+++ b/modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java
@@ -256,6 +256,73 @@
  * <p>See the {@link Cell} class documentation for a more complete
  * description of how to write custom Cells.
  *
+ * <h4>Warning: Nodes should not be inserted directly into the TableView cells</h4>
+ * {@code TableView} allows for it's cells to contain elements of any type, including
+ * {@link Node} instances. Putting nodes into
+ * the TableView cells is <strong>strongly discouraged</strong>, as it can
+ * lead to unexpected results.
+ *
+ * <p>Important points to note:
+ * <ul>
+ * <li>Avoid inserting {@code Node} instances directly into the {@code TableView} cells or its data model.</li>
+ * <li>The recommended approach is to put the relevant information into the items list, and
+ * provide a custom {@link TableColumn#cellFactoryProperty() cell factory} to create the nodes for a
+ * given cell and update them on demand using the data stored in the item for that cell.</li>
+ * <li>Avoid creating new {@code Node}s in the {@code updateItem} method of a custom {@link TableColumn#cellFactoryProperty() cell factory}.</li>
+ * </ul>
+ * <p>The following minimal example shows how to create a custom cell factory for {@code TableView} containing {@code Node}s:
+ * <pre> {@code
+ *  class CustomColor {
+ *    private SimpleObjectProperty<Color> color;
+ *
+ *    CustomColor(Color col) {
+ *      this.color = new SimpleObjectProperty<Color>(col);
+ *    }
+ *    public Color getColor() { return color.getValue(); }
+ *    public void setColor(Color c) { color.setValue(c); }
+ *    public SimpleObjectProperty<Color> colorProperty() { return color; }
+ *  }
+ *
+ *  TableView<CustomColor> tableview = new TableView<CustomColor>();
+ *
+ *  ObservableList<CustomColor> colorList = FXCollections.observableArrayList();
+ *  colorList.addAll(
+ *      new CustomColor(Color.RED),
+ *      new CustomColor(Color.GREEN),
+ *      new CustomColor(Color.BLUE));
+ *
+ *  TableColumn<CustomColor, Color> col = new TableColumn<CustomColor, Color>("Color");
+ *  col.setCellValueFactory(data -> data.getValue().colorProperty());
+ *
+ *  col.setCellFactory(p -> {
+ *    return new TableCell<CustomColor, Color> () {
+ *        private final Rectangle rectangle;
+ *        {
+ *            setContentDisplay(ContentDisplay.GRAPHIC_ONLY);
+ *            rectangle = new Rectangle(10, 10);
+ *        }
+ *
+ *        @Override
+ *        protected void updateItem(Color item, boolean empty) {
+ *          super.updateItem(item, empty);
+ *
+ *          if (item == null || empty) {
+ *              setGraphic(null);
+ *          } else {
+ *              rectangle.setFill(item);
+ *              setGraphic(rectangle);
+ *          }
+ *        }
+ *     };
+ *  });
+ *
+ *  tableview.getColumns().add(col);
+ *  tableview.setItems(colorList); }</pre>
+ *
+ * <p> This example has an anonymous custom {@code TableCell} class in the custom cell factory.
+ * Note that the {@code Rectangle} ({@code Node}) object needs to be created in the instance initialization block
+ * or the constructor of the custom {@code TableCell} class and updated/used in its {@code updateItem} method.
+ *
  * <h3>Sorting</h3>
  * <p>Prior to JavaFX 8.0, the TableView control would treat the
  * {@link #getItems() items} list as the view model, meaning that any changes to
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java b/modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java
index 1000123a5c5..8cedf4ce984 100644
--- a/modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java
+++ b/modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java
@@ -283,6 +283,84 @@
  * <p>See the {@link Cell} class documentation for a more complete
  * description of how to write custom Cells.
  *
+ * <h4>Warning: Nodes should not be inserted directly into the TreeTableView cells</h4>
+ * {@code TreeTableView} allows for it's cells to contain elements of any type, including
+ * {@link Node} instances. Putting nodes into
+ * the TreeTableView cells is <strong>strongly discouraged</strong>, as it can
+ * lead to unexpected results.
+ *
+ * <p>Important points to note:
+ * <ul>
+ * <li>Avoid inserting {@code Node} instances directly into the {@code TreeTableView} cells or its data model.</li>
+ * <li>The recommended approach is to put the relevant information into the items list, and
+ * provide a custom {@link TreeTableColumn#cellFactoryProperty() cell factory} to create the nodes for a
+ * given cell and update them on demand using the data stored in the item for that cell.</li>
+ * <li>Avoid creating new {@code Node}s in the {@code updateItem} method of a custom {@link TreeTableColumn#cellFactoryProperty() cell factory}.</li>
+ * </ul>
+ * <p>The following minimal example shows how to create a custom cell factory for {@code TreeTableView} containing {@code Node}s:
+ * <pre> {@code
+ *  class ColorModel {
+ *    private SimpleObjectProperty<Color> color;
+ *    private StringProperty name;
+ *
+ *    public ColorModel (String name, Color col) {
+ *      this.color = new SimpleObjectProperty<Color>(col);
+ *      this.name = new SimpleStringProperty(name);
+ *    }
+ *
+ *    public Color getColor() { return color.getValue(); }
+ *    public void setColor(Color c) { color.setValue(c); }
+ *    public SimpleObjectProperty<Color> colorProperty() { return color; }
+ *
+ *    public String getName() { return name.getValue(); }
+ *    public void setName(String s) { name.setValue(s); }
+ *    public StringProperty nameProperty() { return name; }
+ *  }
+ *
+ *  ColorModel rootModel = new ColorModel("Color", Color.WHITE);
+ *  TreeItem<ColorModel> treeRoot = new TreeItem<ColorModel>(rootModel);
+ *  treeRoot.setExpanded(true);
+ *  treeRoot.getChildren().addAll(
+ *      new TreeItem<ColorModel>(new ColorModel("Red", Color.RED)),
+ *      new TreeItem<ColorModel>(new ColorModel("Green", Color.GREEN)),
+ *      new TreeItem<ColorModel>(new ColorModel("Blue", Color.BLUE)));
+ *
+ *  TreeTableView<ColorModel> treeTable = new TreeTableView<ColorModel>(treeRoot);
+ *
+ *  TreeTableColumn<ColorModel, String> nameCol = new TreeTableColumn<>("Color Name");
+ *  TreeTableColumn<ColorModel, Color> colorCol = new TreeTableColumn<>("Color");
+ *
+ *  treeTable.getColumns().setAll(nameCol, colorCol);
+ *
+ *  colorCol.setCellValueFactory(p -> p.getValue().getValue().colorProperty());
+ *  nameCol.setCellValueFactory(p -> p.getValue().getValue().nameProperty());
+ *
+ *  colorCol.setCellFactory(p -> {
+ *      return new TreeTableCell<ColorModel, Color> () {
+ *          private final Rectangle rectangle;
+ *          {
+ *              setContentDisplay(ContentDisplay.GRAPHIC_ONLY);
+ *              rectangle = new Rectangle(10, 10);
+ *          }
+ *
+ *          @Override
+ *          protected void updateItem(Color item, boolean empty) {
+ *              super.updateItem(item, empty);
+ *
+ *              if (item == null || empty) {
+ *                  setGraphic(null);
+ *              } else {
+ *                  rectangle.setFill(item);
+ *                  setGraphic(rectangle);
+ *              }
+ *          }
+ *      };
+ *  });}</pre>
+ *
+ * <p> This example has an anonymous custom {@code TreeTableCell} class in the custom cell factory.
+ * Note that the {@code Rectangle} ({@code Node}) object needs to be created in the instance initialization block
+ * or the constructor of the custom {@code TreeTableCell} class and updated/used in its {@code updateItem} method.
+ *
  * <h3>Editing</h3>
  * <p>This control supports inline editing of values, and this section attempts to
  * give an overview of the available APIs and how you should use them.</p>
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java b/modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java
index 5ff6183f589..050828f8eec 100644
--- a/modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java
+++ b/modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -128,6 +128,55 @@
  * TreeView. See the {@link Cell} class documentation for a more complete
  * description of how to write custom Cells.
  *
+ * <h3>Warning: Nodes should not be inserted directly into the TreeView cells</h3>
+ * {@code TreeView} allows for it's cells to contain elements of any type, including
+ * {@code Node} instances. Putting nodes into
+ * the TreeView cells is <strong>strongly discouraged</strong>, as it can
+ * lead to unexpected results.
+ * <p>Important points to note:
+ * <ul>
+ * <li>Avoid inserting {@code Node} instances directly into the {@code TreeView} cells or its data model.</li>
+ * <li>The recommended approach is to put the relevant information into the items list, and
+ * provide a custom {@link #cellFactoryProperty() cell factory} to create the nodes for a
+ * given cell and update them on demand using the data stored in the item for that cell.</li>
+ * <li>Avoid creating new {@code Node}s in the {@code updateItem} method of a custom {@link #cellFactoryProperty() cell factory}.</li>
+ * </ul>
+ * <p>The following minimal example shows how to create a custom cell factory for {@code TreeView} containing {@code Node}s:
+ *
+ * <pre> {@code  TreeItem<Color> treeRoot = new TreeItem<>();
+ *  treeRoot.setExpanded(true);
+ *  TreeView<Color> treeView = new TreeView<>(treeRoot);
+ *
+ *  treeRoot.getChildren().addAll(
+ *      new TreeItem<>(Color.RED),
+ *      new TreeItem<>(Color.GREEN),
+ *      new TreeItem<>(Color.BLUE));
+ *
+ *  treeView.setCellFactory(p -> {
+ *      return new TreeCell<Color>() {
+ *      private final Rectangle rectangle;
+ *      {
+ *          setContentDisplay(ContentDisplay.GRAPHIC_ONLY);
+ *          rectangle = new Rectangle(10, 10);
+ *      }
+ *
+ *      @Override
+ *      protected void updateItem(Color item, boolean empty) {
+ *          super.updateItem(item, empty);
+ *
+ *          if (item == null || empty) {
+ *              setGraphic(null);
+ *          } else {
+ *              rectangle.setFill(item);
+ *              setGraphic(rectangle);
+ *          }
+ *      }
+ *  };});}</pre>
+ *
+ * <p> This example has an anonymous custom {@code TreeCell} class in the custom cell factory.
+ * Note that the {@code Rectangle} ({@code Node}) object needs to be created in the instance initialization block
+ * or the constructor of the custom {@code TreeCell} class and updated/used in its {@code updateItem} method.
+ *
  * <h2>Editing</h2>
  * <p>This control supports inline editing of values, and this section attempts to
  * give an overview of the available APIs and how you should use them.</p>