Toggle enable status of menu items

src/main/java/org/jabref/gui/JabRefFrame.java

@@ -259,8 +259,8 @@ public void init() {
             }
 
             // Poor-mans binding to global state
-            // We need to invoke this in the JavaFX thread as all the listeners sit there
-            Platform.runLater(() -> Globals.stateManager.activeDatabaseProperty().setValue(Optional.of(currentBasePanel.getBibDatabaseContext())));
+            Globals.stateManager.activeDatabaseProperty().setValue(Optional.of(currentBasePanel.getBibDatabaseContext()));
+            Globals.stateManager.setSelectedEntries(currentBasePanel.getSelectedEntries());
 
             // Update search query
             String content = "";
@@ -728,7 +728,7 @@ private MenuBar createMenu() {
 
                 new SeparatorMenuItem(),
 
-                factory.createMenuItem(StandardActions.MANAGE_KEYWORDS, new ManageKeywordsAction(this))
+                factory.createMenuItem(StandardActions.MANAGE_KEYWORDS, new ManageKeywordsAction(Globals.stateManager))
         );
 
         if (Globals.prefs.getBoolean(JabRefPreferences.SPECIALFIELDSENABLED)) {

src/main/java/org/jabref/gui/StateManager.java

@@ -6,23 +6,19 @@
 import java.util.stream.Collectors;
 
 import javafx.beans.binding.Bindings;
-import javafx.beans.property.ObjectProperty;
 import javafx.beans.property.ReadOnlyListProperty;
 import javafx.beans.property.ReadOnlyListWrapper;
-import javafx.beans.property.SimpleObjectProperty;
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
 import javafx.collections.ObservableMap;
 
+import org.jabref.gui.util.OptionalObjectProperty;
 import org.jabref.logic.search.SearchQuery;
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
 import org.jabref.model.groups.GroupTreeNode;
 import org.jabref.model.util.OptionalUtil;
 
-import org.fxmisc.easybind.EasyBind;
-import org.fxmisc.easybind.monadic.MonadicBinding;
-
 /**
  * This class manages the GUI-state of JabRef, including:
  * - currently selected database
@@ -33,22 +29,21 @@
  */
 public class StateManager {
 
-    private final ObjectProperty<Optional<BibDatabaseContext>> activeDatabase = new SimpleObjectProperty<>(Optional.empty());
+    private final OptionalObjectProperty<BibDatabaseContext> activeDatabase = OptionalObjectProperty.empty();
     private final ReadOnlyListWrapper<GroupTreeNode> activeGroups = new ReadOnlyListWrapper<>(FXCollections.observableArrayList());
     private final ObservableList<BibEntry> selectedEntries = FXCollections.observableArrayList();
     private final ObservableMap<BibDatabaseContext, ObservableList<GroupTreeNode>> selectedGroups = FXCollections.observableHashMap();
-    private final ObjectProperty<Optional<SearchQuery>> activeSearchQuery = new SimpleObjectProperty<>(Optional.empty());
+    private final OptionalObjectProperty<SearchQuery> activeSearchQuery = OptionalObjectProperty.empty();
 
     public StateManager() {
-        MonadicBinding<BibDatabaseContext> currentDatabase = EasyBind.map(activeDatabase, database -> database.orElse(null));
-        activeGroups.bind(Bindings.valueAt(selectedGroups, currentDatabase));
+        activeGroups.bind(Bindings.valueAt(selectedGroups, activeDatabase.orElse(null)));
     }
 
-    public ObjectProperty<Optional<BibDatabaseContext>> activeDatabaseProperty() {
+    public OptionalObjectProperty<BibDatabaseContext> activeDatabaseProperty() {
         return activeDatabase;
     }
 
-    public ObjectProperty<Optional<SearchQuery>> activeSearchQueryProperty() {
+    public OptionalObjectProperty<SearchQuery> activeSearchQueryProperty() {
         return activeSearchQuery;
     }
 

src/main/java/org/jabref/gui/actions/ActionFactory.java

@@ -1,24 +1,35 @@
 package org.jabref.gui.actions;
 
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.util.Objects;
 
 import javafx.scene.Node;
 import javafx.scene.control.Button;
 import javafx.scene.control.ButtonBase;
 import javafx.scene.control.CheckMenuItem;
+import javafx.scene.control.Label;
 import javafx.scene.control.Menu;
 import javafx.scene.control.MenuItem;
+import javafx.scene.control.Tooltip;
 
 import org.jabref.gui.keyboard.KeyBindingRepository;
+import org.jabref.model.strings.StringUtil;
 
+import com.sun.javafx.scene.control.skin.ContextMenuContent;
 import de.saxsys.mvvmfx.utils.commands.Command;
 import org.controlsfx.control.action.ActionUtils;
+import org.fxmisc.easybind.EasyBind;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Helper class to create and style controls according to an {@link Action}.
  */
 public class ActionFactory {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger(ActionFactory.class);
+
     private final KeyBindingRepository keyBindingRepository;
 
     public ActionFactory(KeyBindingRepository keyBindingRepository) {
@@ -33,14 +44,64 @@ private static void setGraphic(MenuItem node, Action action) {
         action.getIcon().ifPresent(icon -> node.setGraphic(icon.getGraphicNode()));
     }
 
-    public MenuItem configureMenuItem(Action action, Command command, MenuItem menuItem) {
-        return ActionUtils.configureMenuItem(new JabRefAction(action, command, keyBindingRepository), menuItem);
+    /*
+     * Returns MenuItemContainer node associated with this menu item
+     * which can contain:
+     *   1. label node of type Label for displaying menu item text,
+     *   2. right node of type Label for displaying accelerator text,
+     *      or an arrow if it's a Menu,
+     *   3. graphic node for displaying menu item icon, and
+     *   4. left node for displaying either radio button or check box.
+     *
+     * This is basically rewritten impl_styleableGetNode() which
+     * should not be used since it's marked as deprecated.
+     */
+    private static Label getAssociatedNode(MenuItem menuItem) {
+        ContextMenuContent.MenuItemContainer container = (ContextMenuContent.MenuItemContainer) menuItem.impl_styleableGetNode();
+
+        if (container == null) {
+            return null;
+        } else {
+            // We have to use reflection to get the associated label
+            try {
+                Method getLabel = ContextMenuContent.MenuItemContainer.class.getDeclaredMethod("getLabel");
+                getLabel.setAccessible(true);
+                return (Label) getLabel.invoke(container);
+            } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) {
+                LOGGER.warn("Could not get label of menu item", e);
+            }
+        }
+        return null;
     }
 
-    public MenuItem createMenuItem(Action action, Command command) {
-        MenuItem menuItem = ActionUtils.createMenuItem(new JabRefAction(action, command, keyBindingRepository));
+    public MenuItem configureMenuItem(Action action, Command command, MenuItem menuItem) {
+        ActionUtils.configureMenuItem(new JabRefAction(action, command, keyBindingRepository), menuItem);
         setGraphic(menuItem, action);
 
+        // Show tooltips
+        if (command instanceof SimpleCommand) {
+            EasyBind.subscribe(
+                    ((SimpleCommand) command).statusMessageProperty(),
+                    message -> {
+                        Label label = getAssociatedNode(menuItem);
+                        if (label != null) {
+                            label.setMouseTransparent(false);
+                            if (StringUtil.isBlank(message)) {
+                                label.setTooltip(null);
+                            } else {
+                                label.setTooltip(new Tooltip(message));
+                            }
+                        }
+                    }
+            );
+        }
+
+        return menuItem;
+    }
+
+    public MenuItem createMenuItem(Action action, Command command) {
+        MenuItem menuItem = new MenuItem();
+        configureMenuItem(action, command, menuItem);
         return menuItem;
     }
 

src/main/java/org/jabref/gui/actions/ActionHelper.java

@@ -0,0 +1,16 @@
+package org.jabref.gui.actions;
+
+import javafx.beans.binding.Bindings;
+import javafx.beans.binding.BooleanExpression;
+
+import org.jabref.gui.StateManager;
+
+public class ActionHelper {
+    public static BooleanExpression needsDatabase(StateManager stateManager) {
+        return stateManager.activeDatabaseProperty().isPresent();
+    }
+
+    public static BooleanExpression needsEntriesSelected(StateManager stateManager) {
+        return Bindings.isNotEmpty(stateManager.getSelectedEntries());
+    }
+}

src/main/java/org/jabref/gui/actions/JabRefAction.java

@@ -1,5 +1,7 @@
 package org.jabref.gui.actions;
 
+import javafx.beans.binding.Bindings;
+
 import org.jabref.Globals;
 import org.jabref.gui.keyboard.KeyBindingRepository;
 
@@ -10,7 +12,6 @@
  */
 class JabRefAction extends org.controlsfx.control.action.Action {
 
-
     public JabRefAction(Action action, KeyBindingRepository keyBindingRepository) {
         super(action.getText());
         action.getIcon()
@@ -19,22 +20,34 @@ public JabRefAction(Action action, KeyBindingRepository keyBindingRepository) {
               .ifPresent(keyBinding -> setAccelerator(keyBindingRepository.getKeyCombination(keyBinding)));
 
         setLongText(action.getDescription());
-
     }
 
     public JabRefAction(Action action, Command command, KeyBindingRepository keyBindingRepository) {
         this(action, keyBindingRepository);
 
         setEventHandler(event -> {
             command.execute();
-            trackExecute();
+            trackExecute(getActionName(action, command));
         });
 
         disabledProperty().bind(command.executableProperty().not());
+
+        if (command instanceof SimpleCommand) {
+            SimpleCommand ourCommand = (SimpleCommand) command;
+            longTextProperty().bind(Bindings.concat(action.getDescription(), ourCommand.statusMessageProperty()));
+        }
+    }
+
+    private String getActionName(Action action, Command command) {
+        if (command.getClass().isAnonymousClass()) {
+            return action.getText();
+        } else {
+            return command.getClass().getSimpleName();
+        }
     }
 
-    private void trackExecute() {
+    private void trackExecute(String actionName) {
         Globals.getTelemetryClient()
-               .ifPresent(telemetryClient -> telemetryClient.trackEvent(getText()));
+               .ifPresent(telemetryClient -> telemetryClient.trackEvent(actionName));
     }
 }

src/main/java/org/jabref/gui/actions/SimpleCommand.java

@@ -1,6 +1,8 @@
 package org.jabref.gui.actions;
 
 import javafx.beans.property.ReadOnlyDoubleProperty;
+import javafx.beans.property.ReadOnlyStringProperty;
+import javafx.beans.property.ReadOnlyStringWrapper;
 
 import org.jabref.gui.util.BindingsHelper;
 
@@ -10,6 +12,17 @@
  * A simple command that does not track progress of the action.
  */
 public abstract class SimpleCommand extends CommandBase {
+
+    protected ReadOnlyStringWrapper statusMessage = new ReadOnlyStringWrapper("");
+
+    public String getStatusMessage() {
+        return statusMessage.get();
+    }
+
+    public ReadOnlyStringProperty statusMessageProperty() {
+        return statusMessage.getReadOnlyProperty();
+    }
+
     @Override
     public double getProgress() {
         return 0;

src/main/java/org/jabref/gui/edit/ManageKeywordsAction.java

@@ -1,34 +1,31 @@
 package org.jabref.gui.edit;
 
-import org.jabref.gui.BasePanel;
-import org.jabref.gui.JabRefFrame;
+import org.jabref.gui.StateManager;
 import org.jabref.gui.actions.SimpleCommand;
+import org.jabref.gui.util.BindingsHelper;
 import org.jabref.logic.l10n.Localization;
 
+import static org.jabref.gui.actions.ActionHelper.needsDatabase;
+import static org.jabref.gui.actions.ActionHelper.needsEntriesSelected;
+
 /**
  * An Action for launching keyword managing dialog
  *
  */
 public class ManageKeywordsAction extends SimpleCommand {
 
-    private final JabRefFrame frame;
+    private final StateManager stateManager;
+
+    public ManageKeywordsAction(StateManager stateManager) {
+        this.stateManager = stateManager;
 
-    public ManageKeywordsAction(JabRefFrame frame) {
-        this.frame = frame;
+        this.executable.bind(needsDatabase(stateManager).and(needsEntriesSelected(stateManager)));
+        this.statusMessage.bind(BindingsHelper.ifThenElse(this.executable, "", Localization.lang("Select at least one entry to manage keywords.")));
     }
 
     @Override
     public void execute() {
-        BasePanel basePanel = frame.getCurrentBasePanel();
-        if (basePanel == null) {
-            return;
-        }
-        if (basePanel.getSelectedEntries().isEmpty()) {
-            basePanel.output(Localization.lang("Select at least one entry to manage keywords."));
-            return;
-        }
-
-        ManageKeywordsDialog dialog = new ManageKeywordsDialog(basePanel.getSelectedEntries());
+        ManageKeywordsDialog dialog = new ManageKeywordsDialog(stateManager.getSelectedEntries());
         dialog.showAndWait();
     }
 }

src/main/java/org/jabref/gui/util/BindingsHelper.java

@@ -22,6 +22,7 @@
 import javafx.css.PseudoClass;
 import javafx.scene.Node;
 
+import org.fxmisc.easybind.EasyBind;
 import org.fxmisc.easybind.PreboundBinding;
 
 /**
@@ -176,6 +177,16 @@ protected String computeValue() {
         };
     }
 
+    public static <T> ObservableValue<T> ifThenElse(ObservableValue<Boolean> condition, T value, T other) {
+        return EasyBind.map(condition, conditionValue -> {
+            if (conditionValue) {
+                return value;
+            } else {
+                return other;
+            }
+        });
+    }
+
     private static class BidirectionalBinding<A, B> {
 
         private final ObservableValue<A> propertyA;

src/main/java/org/jabref/gui/util/OptionalObjectProperty.java

@@ -0,0 +1,45 @@
+package org.jabref.gui.util;
+
+import java.util.Optional;
+
+import javafx.beans.binding.BooleanExpression;
+import javafx.beans.binding.ObjectBinding;
+import javafx.beans.property.SimpleObjectProperty;
+
+import org.fxmisc.easybind.PreboundBinding;
+
+/**
+ * Similar to {@link org.fxmisc.easybind.monadic.MonadicObservableValue}
+ */
+public class OptionalObjectProperty<T> extends SimpleObjectProperty<Optional<T>> {
+
+    private OptionalObjectProperty(Optional<T> initialValue) {
+        super(initialValue);
+    }
+
+    public static <T> OptionalObjectProperty<T> empty() {
+        return new OptionalObjectProperty<>(Optional.empty());
+    }
+
+    /**
+     * Returns a new ObservableValue that holds the value held by this
+     * ObservableValue, or {@code other} when this ObservableValue is empty.
+     */
+    public ObjectBinding<T> orElse(T other) {
+        return new PreboundBinding<T>(this) {
+            @Override
+            protected T computeValue() {
+                return OptionalObjectProperty.this.getValue().orElse(other);
+            }
+        };
+    }
+
+    public BooleanExpression isPresent() {
+        return BooleanExpression.booleanExpression(new PreboundBinding<Boolean>(this) {
+            @Override
+            protected Boolean computeValue() {
+                return OptionalObjectProperty.this.getValue().isPresent();
+            }
+        });
+    }
+}