Skip to content

Commit

Permalink
8231644: TreeTableView Regression: Indentation wrong using Label as c…
Browse files Browse the repository at this point in the history
…olumn content type

Reviewed-by: aghaisas, kcr
  • Loading branch information
Marius Hanl authored and kevinrushforth committed Sep 29, 2021
1 parent 55faac4 commit 30f5606
Show file tree
Hide file tree
Showing 2 changed files with 210 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,42 @@ public TreeTableCellSkin(TreeTableCell<S,T> control) {
}

/** {@inheritDoc} */
@Override double leftLabelPadding() {
double leftPadding = super.leftLabelPadding();
@Override
protected double computeMinWidth(double height, double topInset, double rightInset, double bottomInset,
double leftInset) {
return super.computeMinWidth(height, topInset, rightInset, bottomInset, leftInset) + calculateIndentation();
}

/** {@inheritDoc} */
@Override
protected void layoutChildren(double x, double y, double w, double h) {
double indentation = calculateIndentation();
x += indentation;
w -= indentation;
super.layoutChildren(x, y, w, h);
}

/** {@inheritDoc} */
@Override
protected double computePrefWidth(double height, double topInset, double rightInset, double bottomInset,
double leftInset) {
if (isDeferToParentForPrefWidth) {
return super.computePrefWidth(height, topInset, rightInset, bottomInset, leftInset) + calculateIndentation();
}

return super.computePrefWidth(height, topInset, rightInset, bottomInset, leftInset);
}

/** {@inheritDoc} */
@Override
protected double computePrefHeight(double width, double topInset, double rightInset, double bottomInset,
double leftInset) {
width -= calculateIndentation();
return super.computePrefHeight(width, topInset, rightInset, bottomInset, leftInset);
}

private double calculateIndentation() {
double indentation = 0;

// RT-27167: we must take into account the disclosure node and the
// indentation (which is not taken into account by the LabeledSkinBase.
Expand All @@ -114,43 +148,43 @@ public TreeTableCellSkin(TreeTableCell<S,T> control) {
TreeTableCell<S,T> cell = getSkinnable();

TreeTableColumn<S,T> tableColumn = cell.getTableColumn();
if (tableColumn == null) return leftPadding;
if (tableColumn == null) return indentation;

// check if this column is the TreeTableView treeColumn (i.e. the
// column showing the disclosure node and graphic).
TreeTableView<S> treeTable = cell.getTreeTableView();
if (treeTable == null) return leftPadding;
if (treeTable == null) return indentation;

int columnIndex = treeTable.getVisibleLeafIndex(tableColumn);

TreeTableColumn<S,?> treeColumn = treeTable.getTreeColumn();
if ((treeColumn == null && columnIndex != 0) || (treeColumn != null && ! tableColumn.equals(treeColumn))) {
return leftPadding;
if ((treeColumn == null && columnIndex != 0) || (treeColumn != null && !tableColumn.equals(treeColumn))) {
return indentation;
}

TreeTableRow<S> treeTableRow = cell.getTableRow();
if (treeTableRow == null) return leftPadding;
if (treeTableRow == null) return indentation;

TreeItem<S> treeItem = treeTableRow.getTreeItem();
if (treeItem == null) return leftPadding;
if (treeItem == null) return indentation;

int nodeLevel = treeTable.getTreeItemLevel(treeItem);
if (! treeTable.isShowRoot()) nodeLevel--;
if (!treeTable.isShowRoot()) nodeLevel--;

double indentPerLevel = 10;
if (treeTableRow.getSkin() instanceof TreeTableRowSkin) {
indentPerLevel = ((TreeTableRowSkin<?>)treeTableRow.getSkin()).getIndentationPerLevel();
}
leftPadding += nodeLevel * indentPerLevel;
indentation += nodeLevel * indentPerLevel;

// add in the width of the disclosure node, if one exists
Map<TableColumnBase<?,?>, Double> mdwp = TableRowSkinBase.maxDisclosureWidthMap;
leftPadding += mdwp.containsKey(treeColumn) ? mdwp.get(treeColumn) : 0;
indentation += mdwp.containsKey(treeColumn) ? mdwp.get(treeColumn) : 0;

// adding in the width of the graphic on the tree item
Node graphic = treeItem.getGraphic();
leftPadding += graphic == null ? 0 : graphic.prefWidth(height);
indentation += graphic == null ? 0 : graphic.prefWidth(height);

return leftPadding;
return indentation;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
/*
* Copyright (c) 2021, 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
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

package test.javafx.scene.control.skin;

import com.sun.javafx.tk.Toolkit;
import javafx.scene.Node;
import javafx.scene.control.Label;
import javafx.scene.control.TreeItem;
import javafx.scene.control.TreeTableCell;
import javafx.scene.control.TreeTableColumn;
import javafx.scene.control.TreeTableRow;
import javafx.scene.control.TreeTableView;
import javafx.scene.control.skin.LabeledSkinBase;
import javafx.scene.control.skin.LabeledSkinBaseShim;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import test.com.sun.javafx.scene.control.infrastructure.StageLoader;
import test.com.sun.javafx.scene.control.infrastructure.VirtualFlowTestUtils;

import java.util.List;

import static org.junit.Assert.assertEquals;

/**
* Tests for the indentation of the {@link javafx.scene.control.TreeTableView}.
*/
public class TreeTableViewIndentationTest {

private TreeTableView<String> treeTableView;
private TreeTableColumn<String, String> column;
private StageLoader stageLoader;

@Before
public void setup() {
treeTableView = new TreeTableView<>();

column = new TreeTableColumn<>("Column");
treeTableView.getColumns().add(column);

TreeItem<String> root = new TreeItem<>("Root");
root.getChildren().addAll(List.of(new TreeItem<>("TreeItem 1"), new TreeItem<>("TreeItem 2")));
treeTableView.setRoot(root);

stageLoader = new StageLoader(treeTableView);
}

@After
public void cleanup() {
stageLoader.dispose();
}

@Test
public void testIndentationOfCell() {
column.setCellFactory(col -> new TreeTableCell<>());

testXCoordinateIsAfterDisclosureNode();
}

@Test
public void testIndentationOfCellWithGraphic() {
column.setCellFactory(col -> new TreeTableCell<>() {

@Override
protected void updateItem(String item, boolean empty) {
super.updateItem(item, empty);

if (empty) {
setGraphic(null);
} else {
setGraphic(new Label("Graphic"));
}
}
});

testXCoordinateIsAfterDisclosureNode();
}

@Test
public void testIndentationOfCellWithText() {
column.setCellFactory(col -> new TreeTableCell<>() {

@Override
protected void updateItem(String item, boolean empty) {
super.updateItem(item, empty);

if (empty) {
setText(null);
} else {
setText("Text");
}
}
});

testXCoordinateIsAfterDisclosureNode();
}

@Test
public void testIndentationOfCellWithGraphicAndText() {
column.setCellFactory(col -> new TreeTableCell<>() {

@Override
protected void updateItem(String item, boolean empty) {
super.updateItem(item, empty);

if (empty) {
setGraphic(null);
setText(null);
} else {
setGraphic(new Label("Graphic"));
setText("Text");
}
}
});

testXCoordinateIsAfterDisclosureNode();
}

private void testXCoordinateIsAfterDisclosureNode() {
Toolkit.getToolkit().firePulse();

TreeTableRow<String> row = (TreeTableRow<String>) VirtualFlowTestUtils.getCell(treeTableView, 0);
TreeTableCell<String, String> cell = (TreeTableCell<String, String>) row.getChildrenUnmodifiable().get(1);

Node graphic = cell.getGraphic();
double x;
if (graphic != null) {
x = graphic.getLayoutX();
} else {
x = LabeledSkinBaseShim.get_text((LabeledSkinBase<TreeTableCell>) cell.getSkin()).getLayoutX();
}

double leftInset = cell.snappedLeftInset();
double disclosureNodeWidth = row.getDisclosureNode().prefWidth(-1);
double expectedX = leftInset + disclosureNodeWidth;

assertEquals(expectedX, x, 0);
}

}

1 comment on commit 30f5606

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.