Skip to content

Commit 6c1a0ca

Browse files
Robert Lichtenbergerkevinrushforth
Robert Lichtenberger
authored and
kevinrushforth
committed
8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS
Reviewed-by: kcr, aghaisas
1 parent c1b14de commit 6c1a0ca

File tree

3 files changed

+178
-28
lines changed

3 files changed

+178
-28
lines changed

‎modules/javafx.controls/src/main/java/javafx/scene/control/ContextMenu.java

Lines changed: 57 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,15 @@
3333
import javafx.event.ActionEvent;
3434
import javafx.event.Event;
3535
import javafx.event.EventHandler;
36-
import javafx.geometry.HPos;
37-
import javafx.geometry.Point2D;
36+
import javafx.geometry.Bounds;
37+
import javafx.geometry.NodeOrientation;
3838
import javafx.geometry.Side;
39-
import javafx.geometry.VPos;
4039
import javafx.scene.Node;
4140
import javafx.scene.Scene;
4241
import javafx.stage.Window;
4342

44-
import com.sun.javafx.util.Utils;
4543
import com.sun.javafx.collections.TrackableObservableList;
4644
import javafx.scene.control.skin.ContextMenuSkin;
47-
import javafx.beans.property.BooleanProperty;
48-
import javafx.beans.property.SimpleBooleanProperty;
4945

5046
/**
5147
* <p>
@@ -226,18 +222,15 @@ public String getName() {
226222

227223
/**
228224
* Shows the {@code ContextMenu} relative to the given anchor node, on the side
229-
* specified by the {@code hpos} and {@code vpos} parameters, and offset
225+
* specified by the {@code side} parameter, and offset
230226
* by the given {@code dx} and {@code dy} values for the x-axis and y-axis, respectively.
231227
* If there is not enough room, the menu is moved to the opposite side and
232228
* the offset is not applied.
233229
* <p>
234-
* To clarify the purpose of the {@code hpos} and {@code vpos} parameters,
235-
* consider that they are relative to the anchor node. As such, a {@code hpos}
236-
* and {@code vpos} of {@code CENTER} would mean that the ContextMenu appears
237-
* on top of the anchor, with the (0,0) position of the {@code ContextMenu}
238-
* positioned at (0,0) of the anchor. A {@code hpos} of right would then shift
239-
* the {@code ContextMenu} such that its top-left (0,0) position would be attached
240-
* to the top-right position of the anchor.
230+
* To clarify the purpose of the {@code side} parameter,
231+
* consider that it is relative to the anchor node.
232+
* As such, a {@code side} of {@code TOP} would mean that the ContextMenu's bottom left corner
233+
* is set to the top left corner of the anchor.
241234
* <p>
242235
* This function is useful for finely tuning the position of a menu,
243236
* relative to the parent node to ensure close alignment.
@@ -246,26 +239,62 @@ public String getName() {
246239
* @param dx the dx value for the x-axis
247240
* @param dy the dy value for the y-axis
248241
*/
249-
// TODO provide more detail
250242
public void show(Node anchor, Side side, double dx, double dy) {
251243
if (anchor == null) return;
252244
if (getItems().size() == 0) return;
253245

254246
getScene().setNodeOrientation(anchor.getEffectiveNodeOrientation());
255-
// FIXME because Side is not yet in javafx.geometry, we have to convert
256-
// to the old HPos/VPos API here, as Utils can not refer to Side in the
257-
// charting API.
258-
HPos hpos = side == Side.LEFT ? HPos.LEFT : side == Side.RIGHT ? HPos.RIGHT : HPos.CENTER;
259-
VPos vpos = side == Side.TOP ? VPos.TOP : side == Side.BOTTOM ? VPos.BOTTOM : VPos.CENTER;
260-
261-
// translate from anchor/hpos/vpos/dx/dy into screenX/screenY
262-
Point2D point = Utils.pointRelativeTo(anchor,
263-
prefWidth(-1), prefHeight(-1),
264-
hpos, vpos, dx, dy, true);
265-
doShow(anchor, point.getX(), point.getY());
247+
setAnchorLocation(getAnchorLocation(side, anchor.getEffectiveNodeOrientation()));
248+
Bounds anchorBounds = anchor.localToScreen(anchor.getLayoutBounds());
249+
double x = getXBySide(anchorBounds, side, anchor.getEffectiveNodeOrientation()) + dx;
250+
double y = getYBySide(anchorBounds, side) + dy;
251+
doShow(anchor, x, y);
266252
}
267253

268-
/**
254+
private AnchorLocation getAnchorLocation(Side side, NodeOrientation orientation) {
255+
if (orientation == NodeOrientation.RIGHT_TO_LEFT) {
256+
switch (side) {
257+
case TOP: return AnchorLocation.CONTENT_BOTTOM_RIGHT;
258+
case RIGHT: return AnchorLocation.CONTENT_TOP_RIGHT;
259+
case BOTTOM: return AnchorLocation.CONTENT_TOP_RIGHT;
260+
case LEFT: return AnchorLocation.CONTENT_TOP_LEFT;
261+
}
262+
} else {
263+
switch (side) {
264+
case TOP: return AnchorLocation.CONTENT_BOTTOM_LEFT;
265+
case RIGHT: return AnchorLocation.CONTENT_TOP_LEFT;
266+
case BOTTOM: return AnchorLocation.CONTENT_TOP_LEFT;
267+
case LEFT: return AnchorLocation.CONTENT_TOP_RIGHT;
268+
}
269+
}
270+
return AnchorLocation.CONTENT_TOP_LEFT; // never reached
271+
}
272+
273+
private double getXBySide(Bounds anchorBounds, Side side, NodeOrientation orientation) {
274+
if (orientation == NodeOrientation.RIGHT_TO_LEFT) {
275+
if (side == Side.RIGHT) {
276+
return anchorBounds.getMinX();
277+
} else {
278+
return anchorBounds.getMaxX();
279+
}
280+
} else {
281+
if (side == Side.RIGHT) {
282+
return anchorBounds.getMaxX();
283+
} else {
284+
return anchorBounds.getMinX();
285+
}
286+
}
287+
}
288+
289+
private double getYBySide(Bounds anchorBounds, Side side) {
290+
if (side == Side.BOTTOM) {
291+
return anchorBounds.getMaxY();
292+
} else {
293+
return anchorBounds.getMinY();
294+
}
295+
}
296+
297+
/**
269298
* Shows the {@code ContextMenu} at the specified screen coordinates. If there
270299
* is not enough room at the specified location to show the {@code ContextMenu}
271300
* given its size requirements, the necessary adjustments are made to bring
@@ -280,6 +309,7 @@ public void show(Node anchor, double screenX, double screenY) {
280309
if (anchor == null) return;
281310
if (getItems().size() == 0) return;
282311
getScene().setNodeOrientation(anchor.getEffectiveNodeOrientation());
312+
setAnchorLocation(AnchorLocation.CONTENT_TOP_LEFT);
283313
doShow(anchor, screenX, screenY);
284314
}
285315

‎modules/javafx.controls/src/test/java/test/javafx/scene/control/ContextMenuTest.java

Lines changed: 120 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@
3030
import test.com.sun.javafx.scene.control.infrastructure.StageLoader;
3131
import javafx.event.ActionEvent;
3232
import javafx.event.EventHandler;
33-
33+
import javafx.geometry.Bounds;
34+
import javafx.geometry.NodeOrientation;
3435
import javafx.geometry.Side;
3536
import javafx.scene.control.Button;
3637
import javafx.scene.control.ContextMenu;
@@ -44,6 +45,7 @@
4445

4546
import static org.junit.Assert.*;
4647
import static com.sun.javafx.scene.control.ContextMenuContentShim.*;
48+
4749
import java.util.Optional;
4850
import javafx.scene.Node;
4951
import javafx.scene.input.KeyCode;
@@ -636,4 +638,121 @@ private ContextMenu createContextMenuAndShowSubMenu() {
636638
assertEquals("Expected " + item1.getText() + ", found " + focusedItem.getText(),
637639
item1, focusedItem);
638640
}
641+
642+
@Test public void test_position_showOnScreen() {
643+
ContextMenu cm = createContextMenu(false);
644+
cm.show(anchorBtn, 100, 100);
645+
646+
assertEquals(100, cm.getAnchorX(), 0.0);
647+
assertEquals(100, cm.getAnchorY(), 0.0);
648+
}
649+
650+
@Test public void test_position_showOnTop() throws InterruptedException {
651+
ContextMenu cm = createContextMenu(false);
652+
cm.show(anchorBtn, Side.TOP, 0, 0);
653+
654+
Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
655+
Node cmNode = cm.getScene().getRoot();
656+
Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());
657+
658+
assertEquals(anchorBounds.getMinX(), cmBounds.getMinX(), 0.0);
659+
assertEquals(anchorBounds.getMinY(), cmBounds.getMaxY(), 0.0);
660+
}
661+
662+
@Test public void test_position_showOnTopOffset() throws InterruptedException {
663+
ContextMenu cm = createContextMenu(false);
664+
cm.show(anchorBtn, Side.TOP, 3, 5);
665+
666+
Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
667+
Node cmNode = cm.getScene().getRoot();
668+
Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());
669+
670+
assertEquals(anchorBounds.getMinX() + 3, cmBounds.getMinX(), 0.0);
671+
assertEquals(anchorBounds.getMinY() + 5, cmBounds.getMaxY(), 0.0);
672+
}
673+
674+
@Test public void test_position_withOrientationTop() throws InterruptedException {
675+
ContextMenu cm = createContextMenu(false);
676+
anchorBtn.setNodeOrientation(NodeOrientation.RIGHT_TO_LEFT);
677+
cm.show(anchorBtn, Side.TOP, 0, 0);
678+
679+
Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
680+
Node cmNode = cm.getScene().getRoot();
681+
Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());
682+
683+
assertEquals(anchorBounds.getMaxX(), cmBounds.getMaxX(), 0.0);
684+
assertEquals(anchorBounds.getMinY(), cmBounds.getMaxY(), 0.0);
685+
}
686+
687+
@Test public void test_position_withOrientationLeft() throws InterruptedException {
688+
ContextMenu cm = createContextMenu(false);
689+
anchorBtn.setNodeOrientation(NodeOrientation.RIGHT_TO_LEFT);
690+
cm.show(anchorBtn, Side.LEFT, 0, 0);
691+
692+
Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
693+
Node cmNode = cm.getScene().getRoot();
694+
Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());
695+
696+
assertEquals(anchorBounds.getMaxX(), cmBounds.getMinX(), 0.0);
697+
assertEquals(anchorBounds.getMinY(), cmBounds.getMinY(), 0.0);
698+
}
699+
700+
701+
@Test public void test_position_withCSS() throws InterruptedException {
702+
anchorBtn.getScene().getStylesheets().add(
703+
getClass().getResource("test_position_showOnTopWithCSS.css").toExternalForm()
704+
);
705+
test_position_showOnTop();
706+
test_position_showOnRight();
707+
test_position_showOnLeft();
708+
test_position_showOnBottom();
709+
}
710+
711+
@Test public void test_position_showOnRight() {
712+
ContextMenu cm = createContextMenu(false);
713+
cm.show(anchorBtn, Side.RIGHT, 0, 0);
714+
715+
Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
716+
Node cmNode = cm.getScene().getRoot();
717+
Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());
718+
719+
assertEquals(anchorBounds.getMaxX(), cmBounds.getMinX(), 0.0);
720+
assertEquals(anchorBounds.getMinY(), cmBounds.getMinY(), 0.0);
721+
}
722+
723+
@Test public void test_position_showOnRightOffset() {
724+
ContextMenu cm = createContextMenu(false);
725+
cm.show(anchorBtn, Side.RIGHT, 3, 5);
726+
727+
Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
728+
Node cmNode = cm.getScene().getRoot();
729+
Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());
730+
731+
assertEquals(anchorBounds.getMaxX() + 3, cmBounds.getMinX(), 0.0);
732+
assertEquals(anchorBounds.getMinY() + 5, cmBounds.getMinY(), 0.0);
733+
}
734+
735+
@Test public void test_position_showOnBottom() {
736+
ContextMenu cm = createContextMenu(false);
737+
cm.show(anchorBtn, Side.BOTTOM, 0, 0);
738+
739+
Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
740+
Node cmNode = cm.getScene().getRoot();
741+
Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());
742+
743+
assertEquals(anchorBounds.getMinX(), cmBounds.getMinX(), 0.0);
744+
assertEquals(anchorBounds.getMaxY(), cmBounds.getMinY(), 0.0);
745+
}
746+
747+
@Test public void test_position_showOnLeft() {
748+
ContextMenu cm = createContextMenu(false);
749+
cm.show(anchorBtn, Side.LEFT, 0, 0);
750+
751+
Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
752+
Node cmNode = cm.getScene().getRoot();
753+
Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());
754+
755+
assertEquals(anchorBounds.getMinX(), cmBounds.getMaxX(), 0.0);
756+
assertEquals(anchorBounds.getMinY(), cmBounds.getMinY(), 0.0);
757+
}
639758
}

‎modules/javafx.controls/src/test/resources/test/javafx/scene/control/test_position_showOnTopWithCSS.css

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
.menu-item { -fx-padding: 10px;}

0 commit comments

Comments
 (0)