diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java index a55ec20ff83..d8a5cba6982 100644 --- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java +++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2025, 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 @@ -95,7 +95,6 @@ public TableRowSkin(TableRow control) { setupTableViewListeners(); } - // FIXME: replace listener to fixedCellSize with direct lookup - JDK-8277000 private void setupTableViewListeners() { TableView tableView = getSkinnable().getTableView(); if (tableView == null) { @@ -104,31 +103,22 @@ private void setupTableViewListeners() { setupTableViewListeners(); }); } else { - registerChangeListener(tableView.fixedCellSizeProperty(), e -> { - VirtualFlow> virtualFlow = getVirtualFlow(); - if (virtualFlow != null) { - unregisterChangeListeners(virtualFlow.widthProperty()); - } - - updateCachedFixedSize(); - }); - - updateCachedFixedSize(); + VirtualFlow> virtualFlow = getVirtualFlow(); + if (virtualFlow != null) { + registerChangeListener(virtualFlow.widthProperty(), _ -> requestLayoutWhenFixedCellSizeSet()); + } } } - private void updateCachedFixedSize() { - TableView tableView = getSkinnable().getTableView(); - if (tableView != null) { - fixedCellSize = tableView.getFixedCellSize(); - fixedCellSizeEnabled = fixedCellSize > 0; - - if (fixedCellSizeEnabled) { - VirtualFlow> virtualFlow = getVirtualFlow(); - if (virtualFlow != null) { - registerChangeListener(virtualFlow.widthProperty(), e -> getSkinnable().requestLayout()); - } - } + /** + * When we have a fixed cell size set, we must request layout when the width of the virtual flow changed, + * because we might need to add or remove cells that are now visible or not anymore. + *
+ * See also: JDK-8144500 and JDK-8185887. + */ + private void requestLayoutWhenFixedCellSizeSet() { + if (getFixedCellSize() > 0) { + getSkinnable().requestLayout(); } } @@ -237,6 +227,12 @@ private TableView getTableView() { return getSkinnable().getTableView(); } + @Override + double getFixedCellSize() { + TableView tableView = getTableView(); + return tableView != null ? tableView.getFixedCellSize() : super.getFixedCellSize(); + } + // test-only TableViewSkin getTableViewSkin() { TableView tableView = getSkinnable().getTableView(); diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java index 355c37bf6b5..f876f3e67b0 100644 --- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java +++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 2025, 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 @@ -40,6 +40,7 @@ import javafx.scene.Node; import javafx.scene.Parent; import javafx.scene.control.*; +import javafx.scene.layout.Region; import javafx.util.Duration; import com.sun.javafx.tk.Toolkit; @@ -115,10 +116,6 @@ public abstract class TableRowSkinBase graphicProperty() { if (index < 0/* || row >= itemsProperty().get().size()*/) return; VirtualFlow virtualFlow = getVirtualFlow(); + double fixedCellSize = getFixedCellSize(); for (int column = 0, max = cells.size(); column < max; column++) { R tableCell = cells.get(column); TableColumnBase tableColumn = getTableColumn(tableCell); @@ -311,7 +309,7 @@ protected ObjectProperty graphicProperty() { width = snapSizeX(tableColumn.getWidth()); boolean isVisible = true; - if (fixedCellSizeEnabled) { + if (fixedCellSize > 0) { // we determine if the cell is visible, and if not we have the // ability to take it out of the scenegraph to help improve // performance. However, we only do this when there is a @@ -413,6 +411,10 @@ protected ObjectProperty graphicProperty() { } } + double getFixedCellSize() { + return Region.USE_COMPUTED_SIZE; + } + int getIndentationLevel(C control) { return 0; } @@ -512,7 +514,8 @@ VirtualFlow getVirtualFlow() { /** {@inheritDoc} */ @Override protected double computePrefHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) { - if (fixedCellSizeEnabled) { + double fixedCellSize = getFixedCellSize(); + if (fixedCellSize > 0) { return fixedCellSize; } @@ -545,7 +548,8 @@ VirtualFlow getVirtualFlow() { /** {@inheritDoc} */ @Override protected double computeMinHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) { - if (fixedCellSizeEnabled) { + double fixedCellSize = getFixedCellSize(); + if (fixedCellSize > 0) { return fixedCellSize; } @@ -575,7 +579,8 @@ VirtualFlow getVirtualFlow() { /** {@inheritDoc} */ @Override protected double computeMaxHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) { - if (fixedCellSizeEnabled) { + double fixedCellSize = getFixedCellSize(); + if (fixedCellSize > 0) { return fixedCellSize; } return super.computeMaxHeight(width, topInset, rightInset, bottomInset, leftInset); diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java index aef5a7f4000..fafaff7e48d 100644 --- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java +++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 2025, 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 @@ -106,7 +106,6 @@ public TreeTableRowSkin(TreeTableRow control) { setupTreeTableViewListeners(); } - // FIXME: replace listener to fixedCellSize with direct lookup - JDK-8277000 private void setupTreeTableViewListeners() { TreeTableView treeTableView = getSkinnable().getTreeTableView(); if (treeTableView == null) { @@ -119,32 +118,22 @@ private void setupTreeTableViewListeners() { updateLeafColumns(); }); - registerChangeListener(getTreeTableView().fixedCellSizeProperty(), e -> { - VirtualFlow> virtualFlow = getVirtualFlow(); - if (virtualFlow != null) { - unregisterChangeListeners(virtualFlow.widthProperty()); - } - - updateCachedFixedSize(); - }); - updateCachedFixedSize(); + VirtualFlow> virtualFlow = getVirtualFlow(); + if (virtualFlow != null) { + registerChangeListener(virtualFlow.widthProperty(), _ -> requestLayoutWhenFixedCellSizeSet()); + } } } - private void updateCachedFixedSize() { - if (getSkinnable() != null) { - TreeTableView t = getSkinnable().getTreeTableView(); - if (t != null) { - fixedCellSize = t.getFixedCellSize(); - fixedCellSizeEnabled = fixedCellSize > 0.0; - - if (fixedCellSizeEnabled) { - VirtualFlow> virtualFlow = getTableViewSkin().getVirtualFlow(); - if (virtualFlow != null) { - registerChangeListener(virtualFlow.widthProperty(), ev -> getSkinnable().requestLayout()); - } - } - } + /** + * When we have a fixed cell size set, we must request layout when the width of the virtual flow changed, + * because we might need to add or remove cells that are now visible or not anymore. + *
+ * See also: JDK-8144500 and JDK-8185887. + */ + private void requestLayoutWhenFixedCellSizeSet() { + if (getFixedCellSize() > 0) { + getSkinnable().requestLayout(); } } @@ -319,6 +308,12 @@ private TreeTableView getTreeTableView() { return getSkinnable().getTreeTableView(); } + @Override + double getFixedCellSize() { + TreeTableView treeTableView = getTreeTableView(); + return treeTableView != null ? treeTableView.getFixedCellSize() : super.getFixedCellSize(); + } + private void updateDisclosureNodeAndGraphic() { disclosureNodeDirty = false; diff --git a/modules/javafx.controls/src/shims/java/javafx/scene/control/skin/TableSkinShim.java b/modules/javafx.controls/src/shims/java/javafx/scene/control/skin/TableSkinShim.java index 7f32970abec..176d936e7db 100644 --- a/modules/javafx.controls/src/shims/java/javafx/scene/control/skin/TableSkinShim.java +++ b/modules/javafx.controls/src/shims/java/javafx/scene/control/skin/TableSkinShim.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2025, 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 @@ -117,13 +117,11 @@ public static VirtualFlow getVirtualFlow(TreeTableView table) { //----------------- Tree/TableRowSkin state public static boolean isFixedCellSizeEnabled(TableRow tableRow) { - TableRowSkin skin = (TableRowSkin) tableRow.getSkin(); - return skin.fixedCellSizeEnabled; + return tableRow.getTableView().getFixedCellSize() > 0; } public static boolean isFixedCellSizeEnabled(TreeTableRow tableRow) { - TreeTableRowSkin skin = (TreeTableRowSkin) tableRow.getSkin(); - return skin.fixedCellSizeEnabled; + return tableRow.getTreeTableView().getFixedCellSize() > 0; } public static boolean isDirty(TableRow tableRow) { diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java index 5e1f597426b..467ed53e50b 100644 --- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 2025, 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 @@ -121,7 +121,6 @@ public class SkinCleanupTest { /** * Test access to fixedCellSize via lookup (not listener) */ - @Disabled("JDK-8277000") @Test public void testTreeTableRowFixedCellSizeListener() { TreeTableView tableView = createPersonTreeTable(false); @@ -271,6 +270,37 @@ public void testTreeTableRowFixedCellSizeEnabled() { assertTrue(isFixedCellSizeEnabled(tableRow), "fixed cell size enabled"); } + @Test + public void testTreeTableRowVirtualFlowWidthListenerReplaceSkin() { + TreeTableView tableView = createPersonTreeTable(false); + tableView.setFixedCellSize(24); + showControl(tableView, true); + VirtualFlow flow = getVirtualFlow(tableView); + TreeTableRow tableRow = (TreeTableRow) getCell(tableView, 1); + replaceSkin(tableRow); + Toolkit.getToolkit().firePulse(); + TreeTableRowSkin rowSkin = (TreeTableRowSkin) tableRow.getSkin(); + assertNotNull( + unregisterChangeListeners(rowSkin, flow.widthProperty()), + "row skin must have listener to virtualFlow width"); + } + + /** + * Sanity test: listener to flow's width is registered. + */ + @Test + public void testTreeTableRowVirtualFlowWidthListener() { + TreeTableView tableView = createPersonTreeTable(false); + tableView.setFixedCellSize(24); + showControl(tableView, true); + VirtualFlow flow = getVirtualFlow(tableView); + TreeTableRow tableRow = (TreeTableRow) getCell(tableView, 1); + TreeTableRowSkin rowSkin = (TreeTableRowSkin) tableRow.getSkin(); + assertNotNull( + unregisterChangeListeners(rowSkin, flow.widthProperty()), + "row skin must have listener to virtualFlow width"); + } + @Test public void testTreeTableRowTracksVirtualFlowReplaceSkin() { TreeTableView tableView = createPersonTreeTable(false); @@ -550,7 +580,6 @@ private TreeTableView createPersonTreeTable(boolean installSkin) { /** * Test access to fixedCellSize via lookup (not listener) */ - @Disabled("JDK-8277000") @Test public void testTableRowFixedCellSizeListener() { TableView tableView = createPersonTable(false); @@ -672,6 +701,31 @@ public void testTableRowVirtualFlowWidthListener() { "row skin must have listener to virtualFlow width"); } + @Test + public void testTableRowTracksVirtualFlowReplaceSkin() { + TableView tableView = createPersonTable(false); + showControl(tableView, true); + VirtualFlow flow = getVirtualFlow(tableView); + TableRow tableRow = (TableRow) getCell(tableView, 1); + replaceSkin(tableRow); + Toolkit.getToolkit().firePulse(); + TableRowSkin rowSkin = (TableRowSkin) tableRow.getSkin(); + checkFollowsWidth(flow, (Region) rowSkin.getNode()); + } + + /** + * Sanity test checks that tree table row skin tracks the virtual flow width. + */ + @Test + public void testTableRowTracksVirtualFlowWidth() { + TableView tableView = createPersonTable(false); + showControl(tableView, true); + VirtualFlow flow = getVirtualFlow(tableView); + TableRow tableRow = (TableRow) getCell(tableView, 1); + TableRowSkin rowSkin = (TableRowSkin) tableRow.getSkin(); + checkFollowsWidth(flow, (Region) rowSkin.getNode()); + } + /** * Sanity: children don't pile up with fixed cell size. */