8251480: TableColumnHeader: calc of cell width must respect row styling

Reviewed-by: mhanl, aghaisas

Author: Robert Lichtenberger

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java

@@ -53,6 +53,7 @@
 import javafx.scene.Node;
 import javafx.scene.control.ContextMenu;
 import javafx.scene.control.Label;
+import javafx.scene.control.SkinBase;
 import javafx.scene.control.TableCell;
 import javafx.scene.control.TableColumn;
 import javafx.scene.control.TableColumnBase;
@@ -645,9 +646,9 @@ private <T,S> void resizeColumnToFitContent(TableView<T> tv, TableColumn<T, S> t
             Region r = (Region) n;
             padding = r.snappedLeftInset() + r.snappedRightInset();
         }
-
-        TableRow<T> tableRow = new TableRow<>();
-        tableRow.updateTableView(tv);
+        Callback<TableView<T>, TableRow<T>> rowFactory = tv.getRowFactory();
+        TableRow<T> tableRow = createMeasureRow(tv, tableSkin, rowFactory);
+        ((SkinBase<?>) tableRow.getSkin()).getChildren().add(cell);
 
         int rows = maxRows == -1 ? items.size() : Math.min(items.size(), maxRows);
         double maxWidth = 0;
@@ -660,8 +661,7 @@ private <T,S> void resizeColumnToFitContent(TableView<T> tv, TableColumn<T, S> t
             cell.updateIndex(row);
 
             if ((cell.getText() != null && !cell.getText().isEmpty()) || cell.getGraphic() != null) {
-                tableSkin.getChildren().add(cell);
-                cell.applyCss();
+                tableRow.applyCss();
                 maxWidth = Math.max(maxWidth, cell.prefWidth(-1));
                 tableSkin.getChildren().remove(cell);
             }
@@ -701,6 +701,20 @@ private <T,S> void resizeColumnToFitContent(TableView<T> tv, TableColumn<T, S> t
         }
     }
 
+    private <T> TableRow<T> createMeasureRow(TableView<T> tv, TableViewSkinBase tableSkin,
+            Callback<TableView<T>, TableRow<T>> rowFactory) {
+        TableRow<T> tableRow = rowFactory != null ? rowFactory.call(tv) : new TableRow<>();
+        tableSkin.getChildren().add(tableRow);
+        tableRow.applyCss();
+        if (!(tableRow.getSkin() instanceof SkinBase<?>)) {
+            tableSkin.getChildren().remove(tableRow);
+            // recreate with null rowFactory will result in a standard TableRow that will
+            // have a SkinBase-derived skin
+            tableRow = createMeasureRow(tv, tableSkin, null);
+        }
+        return tableRow;
+    }
+
     private <T,S> void resizeColumnToFitContent(TreeTableView<T> ttv, TreeTableColumn<T, S> tc, TableViewSkinBase tableSkin, int maxRows) {
         List<?> items = new TreeTableViewBackingList(ttv);
         if (items == null || items.isEmpty()) return;

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableColumnHeaderTest.java

@@ -29,12 +29,18 @@
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
 import javafx.event.Event;
+import javafx.scene.Node;
+import javafx.scene.control.Skin;
+import javafx.scene.control.Skinnable;
 import javafx.scene.control.TableColumn;
+import javafx.scene.control.TableRow;
 import javafx.scene.control.TableView;
 import javafx.scene.control.cell.PropertyValueFactory;
 import javafx.scene.control.skin.TableColumnHeader;
 import javafx.scene.input.MouseButton;
 import javafx.scene.input.MouseEvent;
+import javafx.scene.layout.HBox;
+
 import org.junit.Before;
 import org.junit.After;
 import org.junit.Test;
@@ -252,4 +258,76 @@ public void test_resizeColumnToFitContentMaxRow() {
         assertEquals("Width must be equal to initial value",
                 width, column.getWidth(), 0.001);
     }
+
+    /** Row style must affect the required column width */
+    @Test
+    public void test_resizeColumnToFitContentRowStyle() {
+        TableColumn column = tableView.getColumns().get(0);
+
+        tableView.setRowFactory(this::createSmallRow);
+        TableColumnHeaderShim.resizeColumnToFitContent(firstColumnHeader, -1);
+        double width = column.getWidth();
+
+        tableView.setRowFactory(this::createLargeRow);
+        TableColumnHeaderShim.resizeColumnToFitContent(firstColumnHeader, -1);
+        assertTrue("Column width must be greater", width < column.getWidth());
+    }
+
+    /** Test resizeColumnToFitContent in the presence of a non-standard row skin */
+    @Test
+    public void test_resizeColumnToFitContentCustomRowSkin() {
+        TableColumn column = tableView.getColumns().get(0);
+
+        tableView.setRowFactory(this::createCustomRow);
+        TableColumnHeaderShim.resizeColumnToFitContent(firstColumnHeader, -1);
+        double width = column.getWidth();
+        assertTrue(width > 0);
+    }
+
+    private TableRow<Person> createCustomRow(TableView<Person> tableView) {
+        TableRow<Person> row = new TableRow<>() {
+            protected Skin<?> createDefaultSkin() {
+                return new CustomSkin(this);
+            };
+        };
+        return row;
+    }
+
+    private static class CustomSkin implements Skin<TableRow<?>> {
+
+        private TableRow<?> row;
+        private Node node = new HBox();
+
+        CustomSkin(TableRow<?> row) {
+            this.row = row;
+        }
+
+        @Override
+        public TableRow<?> getSkinnable() {
+            return row;
+        }
+
+        @Override
+        public Node getNode() {
+            return node;
+        }
+
+        @Override
+        public void dispose() {
+            node = null;
+        }
+    }
+
+    private TableRow<Person> createSmallRow(TableView<Person> tableView) {
+        TableRow<Person> row = new TableRow<>();
+        row.setStyle("-fx-font: 24 Amble");
+        return row;
+    }
+
+    private TableRow<Person> createLargeRow(TableView<Person> param) {
+        TableRow<Person> row = new TableRow<>();
+        row.setStyle("-fx-font: 48 Amble");
+        return row;
+    }
+
 }