Skip to content

Commit

Permalink
8087673: [TableView] TableView and TreeTableView menu button overlaps…
Browse files Browse the repository at this point in the history 
… columns when using a constrained resize policy.

Reviewed-by: angorya, aghaisas
  • Loading branch information
Jose Pereda committed Sep 5, 2022
1 parent d1a7ebd commit 2baa10e
Show file tree
Hide file tree
Showing 4 changed files with 252 additions and 9 deletions.
15 changes: 12 additions & 3 deletions modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
View file
Expand Up @@ -294,8 +294,6 @@ public TableColumnHeader(final TableColumnBase tc) {
}
};



/* *************************************************************************
* *
* Properties *
Expand Down Expand Up @@ -372,8 +370,9 @@ public final ReadOnlyObjectProperty<TableColumnBase<?,?>> tableColumnProperty()
isSizeDirty = false;
}

double cornerRegionPadding = tableHeaderRow == null ? 0.0 : tableHeaderRow.cornerPadding.get();
double sortWidth = 0;
double w = snapSizeX(getWidth()) - (snappedLeftInset() + snappedRightInset());
double w = snapSizeX(getWidth()) - (snappedLeftInset() + snappedRightInset()) - cornerRegionPadding;
double h = getHeight() - (snappedTopInset() + snappedBottomInset());
double x = w;

Expand Down Expand Up @@ -476,7 +475,17 @@ protected TableHeaderRow getTableHeaderRow() {
}

void setTableHeaderRow(TableHeaderRow thr) {
if (tableHeaderRow != null) {
changeListenerHandler.unregisterChangeListeners(tableHeaderRow.cornerPadding);
}
tableHeaderRow = thr;
if (tableHeaderRow != null) {
changeListenerHandler.registerChangeListener(tableHeaderRow.cornerPadding, o -> {
if (isLastVisibleColumn) {
requestLayout();
}
});
}
updateTableSkin();
}

Expand Down
46 changes: 40 additions & 6 deletions modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableHeaderRow.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2022, 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
Expand Down Expand Up @@ -30,14 +30,17 @@
import javafx.beans.InvalidationListener;
import javafx.beans.WeakInvalidationListener;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.ObjectProperty;
import javafx.beans.property.DoubleProperty;
import javafx.beans.property.ReadOnlyObjectProperty;
import javafx.beans.property.ReadOnlyObjectWrapper;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleObjectProperty;
import javafx.beans.property.SimpleDoubleProperty;
import javafx.beans.property.StringProperty;
import javafx.beans.value.ChangeListener;
import javafx.beans.value.WeakChangeListener;
import javafx.collections.ListChangeListener;
import javafx.collections.WeakListChangeListener;
import javafx.geometry.Bounds;
import javafx.geometry.HPos;
import javafx.geometry.Insets;
import javafx.geometry.Side;
Expand All @@ -46,9 +49,7 @@
import javafx.scene.control.ContextMenu;
import javafx.scene.control.Control;
import javafx.scene.control.Label;
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableColumnBase;
import javafx.scene.layout.Border;
import javafx.scene.layout.Pane;
import javafx.scene.layout.Region;
import javafx.scene.layout.StackPane;
Expand Down Expand Up @@ -108,6 +109,7 @@ public class TableHeaderRow extends StackPane {
* when clicked shows a PopupMenu consisting of all leaf columns.
*/
private Pane cornerRegion;
final DoubleProperty cornerPadding = new SimpleDoubleProperty();

/**
* PopupMenu shown to users to allow for them to hide/show columns in the
Expand Down Expand Up @@ -155,6 +157,8 @@ public class TableHeaderRow extends StackPane {
}
};

private final ChangeListener<Boolean> cornerPaddingListener = (obs, ov, nv) -> updateCornerPadding();

private final WeakInvalidationListener weakTableWidthListener =
new WeakInvalidationListener(tableWidthListener);

Expand All @@ -170,6 +174,9 @@ public class TableHeaderRow extends StackPane {
private final WeakInvalidationListener weakColumnTextListener =
new WeakInvalidationListener(columnTextListener);

private final WeakChangeListener<Boolean> weakCornerPaddingListener =
new WeakChangeListener<>(cornerPaddingListener);



/* *************************************************************************
Expand Down Expand Up @@ -261,6 +268,8 @@ public TableHeaderRow(final TableViewSkinBase skin) {
columnPopupMenu.show(cornerRegion, Side.BOTTOM, 0, 0);
me.consume();
});
cornerRegion.visibleProperty().addListener(weakCornerPaddingListener);
flow.getVbar().visibleProperty().addListener(weakCornerPaddingListener);

// the actual header
// the region that is anchored above the vertical scrollbar
Expand Down Expand Up @@ -381,8 +390,10 @@ private final void setRootHeader(NestedTableColumnHeader value) {
filler.resizeRelocate(x + headerWidth, snappedTopInset(), fillerWidth, prefHeight);
}

// position the top-right rectangle (which sits above the scrollbar)
// position the top-right rectangle (which sits above the scrollbar if visible, or adds padding to the
// header of the last visible column if not)
cornerRegion.resizeRelocate(tableWidth - cornerWidth, snappedTopInset(), cornerWidth, prefHeight);
updateCornerPadding();
}

/** {@inheritDoc} */
Expand Down Expand Up @@ -662,4 +673,27 @@ private boolean isColumnVisibleInHeader(TableColumnBase col, List columns) {

return false;
}

// When the corner region is visible, and the vertical scrollbar is not,
// in case the corner region is over the header of the last
// visible column, if any, we have to consider its width as extra padding
// for that header, to prevent the content of the latter from being partially
// covered.
private void updateCornerPadding() {
double padding = 0.0;
if (cornerRegion.isVisible() && !flow.getVbar().isVisible()) {
double x = cornerRegion.getLayoutX();
padding = getRootHeader().getColumnHeaders().stream()
.filter(header -> header.isLastVisibleColumn)
.findFirst()
.map(header -> {
Bounds bounds = header.localToScene(header.getBoundsInLocal());
return bounds.getMinX() <= x && x < bounds.getMaxX() ?
cornerRegion.getWidth() : 0.0;
})
.orElse(0.0);
}
cornerPadding.set(padding);
}

}
96 changes: 96 additions & 0 deletions modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewTest.java
View file
Expand Up @@ -46,6 +46,9 @@
import javafx.collections.SetChangeListener;
import javafx.css.PseudoClass;
import javafx.scene.Node;
import javafx.scene.control.Slider;
import javafx.scene.layout.Region;
import org.junit.After;
import test.com.sun.javafx.scene.control.infrastructure.ControlTestUtils;
import test.com.sun.javafx.scene.control.infrastructure.KeyEventFirer;
import test.com.sun.javafx.scene.control.infrastructure.KeyModifier;
Expand Down Expand Up @@ -120,6 +123,8 @@ public class TableViewTest {
private TableView.TableViewSelectionModel sm;
private TableView.TableViewFocusModel<String> fm;

private StageLoader stageLoader;

private ObservableList<Person> personTestData;

@Before public void setup() {
Expand All @@ -135,6 +140,12 @@ public class TableViewTest {
new Person("Michael", "Brown", "michael.brown@example.com"));
}

@After
public void cleanup() {
if (stageLoader != null) {
stageLoader.dispose();
}
}

/*********************************************************************
* Tests for the constructors *
Expand Down Expand Up @@ -5848,4 +5859,89 @@ public void testAnchorRemainsWhenAddingMoreItemsBelow() {
sl.dispose();
}

// See JDK-8087673
@Test
public void testTableMenuButtonDoesNotOverlapColumnHeaderGraphic() {
TableView<String> table = new TableView<>();
table.setColumnResizePolicy(TableView.CONSTRAINED_RESIZE_POLICY);
table.setTableMenuButtonVisible(true);
TableColumn<String, String> column = new TableColumn<>();
Slider slider = new Slider();
slider.setValue(100);
column.setGraphic(slider);
table.getColumns().add(column);

stageLoader = new StageLoader(table);

Toolkit.getToolkit().firePulse();

ScrollBar vbar = VirtualFlowTestUtils.getVirtualFlowVerticalScrollbar(table);
assertFalse(vbar.isVisible());

StackPane thumb = (StackPane) slider.lookup(".thumb");
assertNotNull(thumb);
double thumbMaxX = thumb.localToScene(thumb.getLayoutBounds()).getMaxX();

StackPane corner = (StackPane) table.lookup(".show-hide-columns-button");
assertNotNull(corner);
assertTrue(corner.isVisible());
double cornerMinX = corner.localToScene(corner.getLayoutBounds()).getMinX();

// Verify that the slider's thumb is fully visible, and it is not overlapped
// by the corner region
assertTrue(thumbMaxX < cornerMinX);
}

// See JDK-8087673
@Test
public void testTableMenuButtonDoesNotOverlapLastColumnHeader() {
TableView<String> table = new TableView<>();
table.setTableMenuButtonVisible(true);
for (int i = 0; i < 10; i++) {
final TableColumn<String, String> column = new TableColumn<>(i + " ");
column.setCellValueFactory(value -> new SimpleStringProperty(value.getValue()));
table.getColumns().add(column);
}
for (int i = 0; i < 10; i++) {
table.getItems().add(Integer.toString(i));
}

stageLoader = new StageLoader(new Scene(table, 300, 300));

TableColumn<String, ?> lastColumn = table.getColumns().get(9);
lastColumn.setSortType(DESCENDING);
table.getSortOrder().setAll(lastColumn);
Toolkit.getToolkit().firePulse();

TableColumnHeader lastColumnHeader = VirtualFlowTestUtils.getTableColumnHeader(table, lastColumn);
assertNotNull(lastColumnHeader);

Region arrow = (Region) lastColumnHeader.lookup(".arrow");
assertNotNull(arrow);

ScrollBar vbar = VirtualFlowTestUtils.getVirtualFlowVerticalScrollbar(table);
assertFalse(vbar.isVisible());
ScrollBar hbar = VirtualFlowTestUtils.getVirtualFlowHorizontalScrollbar(table);
assertTrue(hbar.isVisible());

table.scrollToColumnIndex(9);

double headerMinX = lastColumnHeader.localToScene(lastColumnHeader.getLayoutBounds()).getMinX();
double headerMaxX = lastColumnHeader.localToScene(lastColumnHeader.getLayoutBounds()).getMaxX();

double arrowMaxX = arrow.localToScene(arrow.getLayoutBounds()).getMaxX();

StackPane corner = (StackPane) table.lookup(".show-hide-columns-button");
assertNotNull(corner);
assertTrue(corner.isVisible());
double cornerMinX = corner.localToScene(corner.getLayoutBounds()).getMinX();

// Verify that the corner region is over the last visible column header
assertTrue(headerMinX < cornerMinX);
assertTrue(cornerMinX < headerMaxX);

// Verify that the arrow is fully visible, and it is not overlapped
// by the corner region
assertTrue(arrowMaxX < cornerMinX);
}
}

1 comment on commit 2baa10e

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Review

Issues

Please sign in to comment.