Skip to content

Commit

Permalink
8228363: ContextMenu.show with side=TOP does not work the first time …
Browse files Browse the repository at this point in the history 
…in the presence of CSS

Reviewed-by: kcr, aghaisas
  • Loading branch information
@kevinrushforth
Robert Lichtenberger authored and kevinrushforth committed Jan 27, 2021
1 parent c1b14de commit 6c1a0ca
Show file tree
Hide file tree
Showing 3 changed files with 178 additions and 28 deletions.
84 changes: 57 additions & 27 deletions modules/javafx.controls/src/main/java/javafx/scene/control/ContextMenu.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,15 @@
import javafx.event.ActionEvent;
import javafx.event.Event;
import javafx.event.EventHandler;
import javafx.geometry.HPos;
import javafx.geometry.Point2D;
import javafx.geometry.Bounds;
import javafx.geometry.NodeOrientation;
import javafx.geometry.Side;
import javafx.geometry.VPos;
import javafx.scene.Node;
import javafx.scene.Scene;
import javafx.stage.Window;

import com.sun.javafx.util.Utils;
import com.sun.javafx.collections.TrackableObservableList;
import javafx.scene.control.skin.ContextMenuSkin;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.SimpleBooleanProperty;

/**
* <p>
Expand Down Expand Up @@ -226,18 +222,15 @@ public String getName() {

/**
* Shows the {@code ContextMenu} relative to the given anchor node, on the side
* specified by the {@code hpos} and {@code vpos} parameters, and offset
* specified by the {@code side} parameter, and offset
* by the given {@code dx} and {@code dy} values for the x-axis and y-axis, respectively.
* If there is not enough room, the menu is moved to the opposite side and
* the offset is not applied.
* <p>
* To clarify the purpose of the {@code hpos} and {@code vpos} parameters,
* consider that they are relative to the anchor node. As such, a {@code hpos}
* and {@code vpos} of {@code CENTER} would mean that the ContextMenu appears
* on top of the anchor, with the (0,0) position of the {@code ContextMenu}
* positioned at (0,0) of the anchor. A {@code hpos} of right would then shift
* the {@code ContextMenu} such that its top-left (0,0) position would be attached
* to the top-right position of the anchor.
* To clarify the purpose of the {@code side} parameter,
* consider that it is relative to the anchor node.
* As such, a {@code side} of {@code TOP} would mean that the ContextMenu's bottom left corner
* is set to the top left corner of the anchor.
* <p>
* This function is useful for finely tuning the position of a menu,
* relative to the parent node to ensure close alignment.
Expand All @@ -246,26 +239,62 @@ public String getName() {
* @param dx the dx value for the x-axis
* @param dy the dy value for the y-axis
*/
// TODO provide more detail
public void show(Node anchor, Side side, double dx, double dy) {
if (anchor == null) return;
if (getItems().size() == 0) return;

getScene().setNodeOrientation(anchor.getEffectiveNodeOrientation());
// FIXME because Side is not yet in javafx.geometry, we have to convert
// to the old HPos/VPos API here, as Utils can not refer to Side in the
// charting API.
HPos hpos = side == Side.LEFT ? HPos.LEFT : side == Side.RIGHT ? HPos.RIGHT : HPos.CENTER;
VPos vpos = side == Side.TOP ? VPos.TOP : side == Side.BOTTOM ? VPos.BOTTOM : VPos.CENTER;

// translate from anchor/hpos/vpos/dx/dy into screenX/screenY
Point2D point = Utils.pointRelativeTo(anchor,
prefWidth(-1), prefHeight(-1),
hpos, vpos, dx, dy, true);
doShow(anchor, point.getX(), point.getY());
setAnchorLocation(getAnchorLocation(side, anchor.getEffectiveNodeOrientation()));
Bounds anchorBounds = anchor.localToScreen(anchor.getLayoutBounds());
double x = getXBySide(anchorBounds, side, anchor.getEffectiveNodeOrientation()) + dx;
double y = getYBySide(anchorBounds, side) + dy;
doShow(anchor, x, y);
}

/**
private AnchorLocation getAnchorLocation(Side side, NodeOrientation orientation) {
if (orientation == NodeOrientation.RIGHT_TO_LEFT) {
switch (side) {
case TOP: return AnchorLocation.CONTENT_BOTTOM_RIGHT;
case RIGHT: return AnchorLocation.CONTENT_TOP_RIGHT;
case BOTTOM: return AnchorLocation.CONTENT_TOP_RIGHT;
case LEFT: return AnchorLocation.CONTENT_TOP_LEFT;
}
} else {
switch (side) {
case TOP: return AnchorLocation.CONTENT_BOTTOM_LEFT;
case RIGHT: return AnchorLocation.CONTENT_TOP_LEFT;
case BOTTOM: return AnchorLocation.CONTENT_TOP_LEFT;
case LEFT: return AnchorLocation.CONTENT_TOP_RIGHT;
}
}
return AnchorLocation.CONTENT_TOP_LEFT; // never reached
}

private double getXBySide(Bounds anchorBounds, Side side, NodeOrientation orientation) {
if (orientation == NodeOrientation.RIGHT_TO_LEFT) {
if (side == Side.RIGHT) {
return anchorBounds.getMinX();
} else {
return anchorBounds.getMaxX();
}
} else {
if (side == Side.RIGHT) {
return anchorBounds.getMaxX();
} else {
return anchorBounds.getMinX();
}
}
}

private double getYBySide(Bounds anchorBounds, Side side) {
if (side == Side.BOTTOM) {
return anchorBounds.getMaxY();
} else {
return anchorBounds.getMinY();
}
}

/**
* Shows the {@code ContextMenu} at the specified screen coordinates. If there
* is not enough room at the specified location to show the {@code ContextMenu}
* given its size requirements, the necessary adjustments are made to bring
Expand All @@ -280,6 +309,7 @@ public void show(Node anchor, double screenX, double screenY) {
if (anchor == null) return;
if (getItems().size() == 0) return;
getScene().setNodeOrientation(anchor.getEffectiveNodeOrientation());
setAnchorLocation(AnchorLocation.CONTENT_TOP_LEFT);
doShow(anchor, screenX, screenY);
}

Expand Down
121 changes: 120 additions & 1 deletion modules/javafx.controls/src/test/java/test/javafx/scene/control/ContextMenuTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
import test.com.sun.javafx.scene.control.infrastructure.StageLoader;
import javafx.event.ActionEvent;
import javafx.event.EventHandler;

import javafx.geometry.Bounds;
import javafx.geometry.NodeOrientation;
import javafx.geometry.Side;
import javafx.scene.control.Button;
import javafx.scene.control.ContextMenu;
Expand All @@ -44,6 +45,7 @@

import static org.junit.Assert.*;
import static com.sun.javafx.scene.control.ContextMenuContentShim.*;

import java.util.Optional;
import javafx.scene.Node;
import javafx.scene.input.KeyCode;
Expand Down Expand Up @@ -636,4 +638,121 @@ private ContextMenu createContextMenuAndShowSubMenu() {
assertEquals("Expected " + item1.getText() + ", found " + focusedItem.getText(),
item1, focusedItem);
}

@Test public void test_position_showOnScreen() {
ContextMenu cm = createContextMenu(false);
cm.show(anchorBtn, 100, 100);

assertEquals(100, cm.getAnchorX(), 0.0);
assertEquals(100, cm.getAnchorY(), 0.0);
}

@Test public void test_position_showOnTop() throws InterruptedException {
ContextMenu cm = createContextMenu(false);
cm.show(anchorBtn, Side.TOP, 0, 0);

Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
Node cmNode = cm.getScene().getRoot();
Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());

assertEquals(anchorBounds.getMinX(), cmBounds.getMinX(), 0.0);
assertEquals(anchorBounds.getMinY(), cmBounds.getMaxY(), 0.0);
}

@Test public void test_position_showOnTopOffset() throws InterruptedException {
ContextMenu cm = createContextMenu(false);
cm.show(anchorBtn, Side.TOP, 3, 5);

Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
Node cmNode = cm.getScene().getRoot();
Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());

assertEquals(anchorBounds.getMinX() + 3, cmBounds.getMinX(), 0.0);
assertEquals(anchorBounds.getMinY() + 5, cmBounds.getMaxY(), 0.0);
}

@Test public void test_position_withOrientationTop() throws InterruptedException {
ContextMenu cm = createContextMenu(false);
anchorBtn.setNodeOrientation(NodeOrientation.RIGHT_TO_LEFT);
cm.show(anchorBtn, Side.TOP, 0, 0);

Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
Node cmNode = cm.getScene().getRoot();
Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());

assertEquals(anchorBounds.getMaxX(), cmBounds.getMaxX(), 0.0);
assertEquals(anchorBounds.getMinY(), cmBounds.getMaxY(), 0.0);
}

@Test public void test_position_withOrientationLeft() throws InterruptedException {
ContextMenu cm = createContextMenu(false);
anchorBtn.setNodeOrientation(NodeOrientation.RIGHT_TO_LEFT);
cm.show(anchorBtn, Side.LEFT, 0, 0);

Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
Node cmNode = cm.getScene().getRoot();
Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());

assertEquals(anchorBounds.getMaxX(), cmBounds.getMinX(), 0.0);
assertEquals(anchorBounds.getMinY(), cmBounds.getMinY(), 0.0);
}


@Test public void test_position_withCSS() throws InterruptedException {
anchorBtn.getScene().getStylesheets().add(
getClass().getResource("test_position_showOnTopWithCSS.css").toExternalForm()
);
test_position_showOnTop();
test_position_showOnRight();
test_position_showOnLeft();
test_position_showOnBottom();
}

@Test public void test_position_showOnRight() {
ContextMenu cm = createContextMenu(false);
cm.show(anchorBtn, Side.RIGHT, 0, 0);

Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
Node cmNode = cm.getScene().getRoot();
Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());

assertEquals(anchorBounds.getMaxX(), cmBounds.getMinX(), 0.0);
assertEquals(anchorBounds.getMinY(), cmBounds.getMinY(), 0.0);
}

@Test public void test_position_showOnRightOffset() {
ContextMenu cm = createContextMenu(false);
cm.show(anchorBtn, Side.RIGHT, 3, 5);

Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
Node cmNode = cm.getScene().getRoot();
Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());

assertEquals(anchorBounds.getMaxX() + 3, cmBounds.getMinX(), 0.0);
assertEquals(anchorBounds.getMinY() + 5, cmBounds.getMinY(), 0.0);
}

@Test public void test_position_showOnBottom() {
ContextMenu cm = createContextMenu(false);
cm.show(anchorBtn, Side.BOTTOM, 0, 0);

Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
Node cmNode = cm.getScene().getRoot();
Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());

assertEquals(anchorBounds.getMinX(), cmBounds.getMinX(), 0.0);
assertEquals(anchorBounds.getMaxY(), cmBounds.getMinY(), 0.0);
}

@Test public void test_position_showOnLeft() {
ContextMenu cm = createContextMenu(false);
cm.show(anchorBtn, Side.LEFT, 0, 0);

Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
Node cmNode = cm.getScene().getRoot();
Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());

assertEquals(anchorBounds.getMinX(), cmBounds.getMaxX(), 0.0);
assertEquals(anchorBounds.getMinY(), cmBounds.getMinY(), 0.0);
}
}
1 change: 1 addition & 0 deletions ....controls/src/test/resources/test/javafx/scene/control/test_position_showOnTopWithCSS.css
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
.menu-item { -fx-padding: 10px;}

1 comment on commit 6c1a0ca

@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.