Merge branch 'issue38-property-popover'

Author: oowekyala

src/main/java/net/sourceforge/pmd/util/fxdesigner/model/PropertyDescriptorSpec.java

@@ -147,8 +147,8 @@ public Var<String> valueProperty() {
      * @return An xml string
      */
     public String toXml() {
-        return String.format("<property name=\"%s\" type=\"%s\" value=\"%s\" />",
-                             getName(), getTypeId().getStringId(), getValue());
+        return String.format("<property name=\"%s\" type=\"%s\" value=\"%s\" description=\"%s\"/>",
+                             getName(), getTypeId().getStringId(), getValue(), getDescription());
     }
 
 

src/main/java/net/sourceforge/pmd/util/fxdesigner/popups/ExportXPathWizardController.java

@@ -8,7 +8,6 @@
 import static com.github.oowekyala.rxstring.ItemRenderer.indented;
 import static com.github.oowekyala.rxstring.ItemRenderer.surrounded;
 import static com.github.oowekyala.rxstring.ItemRenderer.wrapped;
-import static net.sourceforge.pmd.util.fxdesigner.util.AuxLanguageRegistry.getSupportedLanguageVersions;
 import static net.sourceforge.pmd.util.fxdesigner.util.DesignerUtil.stringConverter;
 import static net.sourceforge.pmd.util.fxdesigner.util.reactfx.ReactfxUtil.rewireInit;
 
@@ -173,10 +172,7 @@ public Subscription bindToRuleBuilder(ObservableXPathRuleBuilder ruleBuilder) {
 
 
     private void initialiseLanguageChoiceBox() {
-        languageChoiceBox.getItems().addAll(getSupportedLanguageVersions().stream()
-                                                                          .map(LanguageVersion::getLanguage)
-                                                                          .distinct()
-                                                                          .collect(Collectors.toList()));
+        languageChoiceBox.getItems().setAll(AuxLanguageRegistry.getSupportedLanguages().collect(Collectors.toList()));
 
         languageChoiceBox.setConverter(stringConverter(Language::getShortName, AuxLanguageRegistry::findLanguageByShortName));
     }
@@ -286,6 +282,7 @@ private static LiveTemplateBuilder<ObservableXPathRuleBuilder> liveTemplateBuild
                             .append("<property name=\"").bind(PropertyDescriptorSpec::nameProperty)
                             .append("\" type=\"").bind(PropertyDescriptorSpec::typeIdProperty, PropertyTypeId::getStringId)
                             .append("\" value=\"").bind(PropertyDescriptorSpec::valueProperty)
+                            .append("\" description=\"").bind(PropertyDescriptorSpec::descriptionProperty)
                             .appendLine("\"/>")
             )
             .appendIndent(2).append("<property name=\"version\" value=\"").bind(ObservableXPathRuleBuilder::xpathVersionProperty).appendLine("\"/>")

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

@@ -19,6 +19,7 @@
 import net.sourceforge.pmd.util.fxdesigner.util.DesignerUtil;
 
 import javafx.application.Platform;
+import javafx.event.EventHandler;
 import javafx.stage.Stage;
 import javafx.stage.Window;
 import javafx.stage.WindowEvent;
@@ -34,6 +35,7 @@ public final class PopOverWrapper<T> {
     private final Var<PopOver> myPopover = Var.newSimpleVar(null);
     private BiFunction<T, PopOver, PopOver> rebinder;
     private T identity;
+    private EventHandler<WindowEvent> userOnHiding = we -> {};
 
     public PopOverWrapper(BiFunction<T, @Nullable PopOver, @Nullable PopOver> rebinder) {
         this.identity = null;
@@ -56,7 +58,12 @@ public void showOrFocus(Consumer<@NonNull PopOver> showMethod) {
     }
 
     public void rebind(T identity) {
-        preload(() -> rebinder.apply(identity, myPopover.getValue()));
+        if (identity != this.identity) {
+            this.identity = identity;
+            this.userOnHiding.handle(null);
+            this.userOnHiding = we -> {};
+            preload(() -> rebinder.apply(identity, myPopover.getValue()));
+        }
     }
 
     private void preload(Supplier<PopOver> supplier) {
@@ -68,6 +75,14 @@ private void preload(Supplier<PopOver> supplier) {
             myPopover.setValue(null);
             return;
         }
+
+        this.userOnHiding = popOver.getOnHiding();
+
+        popOver.setOnHiding(we -> {
+            this.identity = null;
+            this.userOnHiding.handle(we);
+        });
+
         popOver.getRoot().getStylesheets().addAll(DesignerUtil.getCss("popover").toString());
         popOver.getRoot().applyCss();
         myPopover.setValue(popOver);

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

@@ -7,7 +7,6 @@
 import java.io.IOException;
 import java.util.Comparator;
 import java.util.Objects;
-import java.util.Optional;
 
 import org.apache.commons.lang3.StringUtils;
 import org.checkerframework.checker.nullness.qual.NonNull;
@@ -62,6 +61,35 @@ public class PropertyCollectionView extends VBox implements ApplicationComponent
         ValueExtractor.addObservableValueExtractor(c -> c instanceof ListCell, c -> ((ListCell) c).itemProperty());
     }
 
+    /*
+     Note: nesting a popover within a popover is error-prone: JavaFX
+     doesn't support a proper focus model for PopupWindows (used by PopOver)
+
+     This means, multiple open popovers maintain independent focus models
+     and react to every event of the stage. Ie there's no stage that has
+     primary focus. Eg if you focus a textfield in a popover and carry
+     on editing somewhere else in the main stage, both places will see
+     the update.
+
+     This also means that when eg here, you click the "add property" button,
+     the button doesn't lose focus in its popover. When editing something in
+     the new popover, this will send an event to the edited text field AND
+     to the "add property" button.
+
+     This is why the action handlers explicitly request focus on some
+     innocuous control (eg listview) after they're done processing.
+
+     This is also why we can't let the edit popover hover around in
+     detached mode.
+
+
+     See:
+
+     https://github.com/openjdk/jfx/blob/be22e8535cb3f98069c0a0216b056cfe8ef037e8/modules/javafx.graphics/src/main/java/javafx/stage/PopupWindow.java#L531-L535
+
+
+     */
+
 
     // for scenebuilder
     @SuppressWarnings("ConstantConditions") // suppress nullability issue
@@ -90,7 +118,10 @@ public PropertyCollectionView(@NamedArg("designerRoot") DesignerRoot root) {
 
         ControlUtil.anchorFirmly(addProperty);
 
-        addProperty.setOnAction(e -> addNewProperty(getUniqueNewName()));
+        addProperty.setOnAction(e -> {
+            addNewProperty(getUniqueNewName());
+            view.requestFocus();
+        });
         footer.getChildren().addAll(addProperty);
         this.getChildren().addAll(view, footer);
 
@@ -175,7 +206,6 @@ private PopOver rebindPopover(PropertyDescriptorSpec newSpec, PopOver pop) {
             // create it
             return detailsPopOver(newSpec);
         }
-        Optional.ofNullable(pop.getOnHiding()).ifPresent(it -> it.handle(null));
 
         pop.titleProperty().bind(newSpec.nameProperty()
                                         .filter(StringUtils::isNotBlank)
@@ -279,6 +309,7 @@ protected Pair<Node, Subscription> getNonEditingGraphic(PropertyDescriptorSpec s
             edit.setOnAction(e -> {
                 myEditPopover.rebind(spec);
                 myEditPopover.showOrFocus(p -> PopOverUtil.showAt(p, owner, this));
+                view.requestFocus();
             });
 
             sub = sub.and(() -> edit.setOnAction(null));
@@ -293,6 +324,7 @@ protected Pair<Node, Subscription> getNonEditingGraphic(PropertyDescriptorSpec s
                     myEditPopover.rebind(null);
                     myEditPopover.hide();
                 }
+                view.requestFocus();
             });
 
             sub = sub.and(() -> delete.setOnAction(null));