Skip to content

Commit fef57ad

Browse files
committed
Fix focus problems with nested popovers
1 parent 797071d commit fef57ad

File tree

1 file changed

+35
-1
lines changed

1 file changed

+35
-1
lines changed

src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/PropertyCollectionView.java

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,35 @@ public class PropertyCollectionView extends VBox implements ApplicationComponent
6161
ValueExtractor.addObservableValueExtractor(c -> c instanceof ListCell, c -> ((ListCell) c).itemProperty());
6262
}
6363

64+
/*
65+
Note: nesting a popover within a popover is error-prone: JavaFX
66+
doesn't support a proper focus model for PopupWindows (used by PopOver)
67+
68+
This means, multiple open popovers maintain independent focus models
69+
and react to every event of the stage. Ie there's no stage that has
70+
primary focus. Eg if you focus a textfield in a popover and carry
71+
on editing somewhere else in the main stage, both places will see
72+
the update.
73+
74+
This also means that when eg here, you click the "add property" button,
75+
the button doesn't lose focus in its popover. When editing something in
76+
the new popover, this will send an event to the edited text field AND
77+
to the "add property" button.
78+
79+
This is why the action handlers explicitly request focus on some
80+
innocuous control (eg listview) after they're done processing.
81+
82+
This is also why we can't let the edit popover hover around in
83+
detached mode.
84+
85+
86+
See:
87+
88+
https://github.com/openjdk/jfx/blob/be22e8535cb3f98069c0a0216b056cfe8ef037e8/modules/javafx.graphics/src/main/java/javafx/stage/PopupWindow.java#L531-L535
89+
90+
91+
*/
92+
6493

6594
// for scenebuilder
6695
@SuppressWarnings("ConstantConditions") // suppress nullability issue
@@ -89,7 +118,10 @@ public PropertyCollectionView(@NamedArg("designerRoot") DesignerRoot root) {
89118

90119
ControlUtil.anchorFirmly(addProperty);
91120

92-
addProperty.setOnAction(e -> addNewProperty(getUniqueNewName()));
121+
addProperty.setOnAction(e -> {
122+
addNewProperty(getUniqueNewName());
123+
view.requestFocus();
124+
});
93125
footer.getChildren().addAll(addProperty);
94126
this.getChildren().addAll(view, footer);
95127

@@ -277,6 +309,7 @@ protected Pair<Node, Subscription> getNonEditingGraphic(PropertyDescriptorSpec s
277309
edit.setOnAction(e -> {
278310
myEditPopover.rebind(spec);
279311
myEditPopover.showOrFocus(p -> PopOverUtil.showAt(p, owner, this));
312+
view.requestFocus();
280313
});
281314

282315
sub = sub.and(() -> edit.setOnAction(null));
@@ -291,6 +324,7 @@ protected Pair<Node, Subscription> getNonEditingGraphic(PropertyDescriptorSpec s
291324
myEditPopover.rebind(null);
292325
myEditPopover.hide();
293326
}
327+
view.requestFocus();
294328
});
295329

296330
sub = sub.and(() -> delete.setOnAction(null));

0 commit comments

Comments
 (0)