Fix closing entry editor with ESC (#2957)

Rename Escape to ESC for javafx Renamed Enum variables as key for the constant was amibiuous
JabRef · Jul 3, 2017 · 6897b10 · 6897b10
1 parent 32a1f22
commit 6897b10
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 21 deletions.
diff --git a/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java b/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java
@@ -178,7 +178,9 @@ public void keyReleased(java.awt.event.KeyEvent e) {
             @Override
             public void keyPressed(java.awt.event.KeyEvent e) {
                 //We need to consume this event here to prevent the propgation of keybinding events back to the JFrame
+
                 e.consume();
+
             }
         });
         DefaultTaskExecutor.runInJavaFXThread(() -> {
@@ -220,9 +222,11 @@ private void setupKeyBindings() {
                     helpAction.actionPerformed(null);
                     event.consume();
                     break;
-
+                case CLOSE_ENTRY_EDITOR:
+                    closeAction.actionPerformed(null);
+                    event.consume();
+                    break;
                 default:
-
                     // Pass other keys to children
                 }
             }

diff --git a/src/main/java/org/jabref/gui/keyboard/KeyBinding.java b/src/main/java/org/jabref/gui/keyboard/KeyBinding.java
@@ -14,7 +14,7 @@ public enum KeyBinding {
     CLEAR_SEARCH("Clear search", Localization.lang("Clear search"), "ESCAPE", KeyBindingCategory.SEARCH),
     CLOSE_DATABASE("Close library", Localization.lang("Close library"), "ctrl+W", KeyBindingCategory.FILE),
     CLOSE_DIALOG("Close dialog", Localization.lang("Close dialog"), "ESCAPE", KeyBindingCategory.FILE),
-    CLOSE_ENTRY_EDITOR("Close entry editor", Localization.lang("Close entry editor"), "ESCAPE", KeyBindingCategory.VIEW),
+    CLOSE_ENTRY_EDITOR("Close entry editor", Localization.lang("Close entry editor"), "Esc", KeyBindingCategory.VIEW),
     COPY("Copy", Localization.lang("Copy"), "ctrl+C", KeyBindingCategory.EDIT),
     COPY_TITLE("Copy title", Localization.lang("Copy title"), "ctrl+shift+alt+T", KeyBindingCategory.EDIT),
     COPY_CITE_BIBTEX_KEY("Copy \\cite{BibTeX key}", Localization.lang("Copy \\cite{BibTeX key}"), "ctrl+K", KeyBindingCategory.EDIT),
@@ -93,27 +93,35 @@ public enum KeyBinding {
     WEB_SEARCH("Web search", Localization.lang("Web search"), "alt+4", KeyBindingCategory.SEARCH),
     WRITE_XMP("Write XMP", Localization.lang("Write XMP"), "F6", KeyBindingCategory.TOOLS);
 
-    private final String key;
+    private final String constant;
     private final String localization;
     private final String defaultBinding;
     private final KeyBindingCategory category;
 
-    KeyBinding(String key, String localization, String defaultBinding, KeyBindingCategory category) {
-        this.key = key;
+    KeyBinding(String constantName, String localization, String defaultKeyBinding, KeyBindingCategory category) {
+        this.constant = constantName;
         this.localization = localization;
-        this.defaultBinding = defaultBinding;
+        this.defaultBinding = defaultKeyBinding;
         this.category = category;
     }
 
-    public String getKey() {
-        return key;
+    /**
+     * This method returns the enum constant value
+     * @return
+     */
+    public String getConstant() {
+        return constant;
     }
 
     public String getLocalization() {
         return localization;
     }
 
-    public String getDefaultBinding() {
+    /**
+     * This method returns the default key binding, the key(s) which are assigned
+     * @return The default key binding
+     */
+    public String getDefaultKeyBinding() {
         return defaultBinding;
     }
 

diff --git a/src/main/java/org/jabref/gui/keyboard/KeyBindingRepository.java b/src/main/java/org/jabref/gui/keyboard/KeyBindingRepository.java
@@ -44,7 +44,7 @@ public KeyBindingRepository(List<String> bindNames, List<String> bindings) {
         if ((bindNames.isEmpty()) || (bindings.isEmpty()) || (bindNames.size() != bindings.size())) {
             // Use default key bindings
             for (KeyBinding keyBinding : KeyBinding.values()) {
-                put(keyBinding, keyBinding.getDefaultBinding());
+                put(keyBinding, keyBinding.getDefaultKeyBinding());
             }
         } else {
             for (int i = 0; i < bindNames.size(); i++) {
@@ -80,7 +80,7 @@ public String get(String key) {
         if (result.isPresent()) {
             return result.get();
         } else if (keyBinding.isPresent()) {
-            return keyBinding.get().getDefaultBinding();
+            return keyBinding.get().getDefaultKeyBinding();
         } else {
             return "Not associated";
         }
@@ -102,15 +102,15 @@ public void put(String key, String value) {
     }
 
     private Optional<KeyBinding> getKeyBinding(String key) {
-        return Arrays.stream(KeyBinding.values()).filter(b -> b.getKey().equals(key)).findFirst();
+        return Arrays.stream(KeyBinding.values()).filter(b -> b.getConstant().equals(key)).findFirst();
     }
 
     public void resetToDefault(String key) {
-        getKeyBinding(key).ifPresent(b -> bindings.put(b, b.getDefaultBinding()));
+        getKeyBinding(key).ifPresent(b -> bindings.put(b, b.getDefaultKeyBinding()));
     }
 
     public void resetToDefault() {
-        bindings.forEach((b, s) -> bindings.put(b, b.getDefaultBinding()));
+        bindings.forEach((b, s) -> bindings.put(b, b.getDefaultKeyBinding()));
     }
 
     public int size() {
@@ -131,7 +131,7 @@ public Optional<KeyBinding> mapToKeyBinding(KeyEvent keyEvent) {
      */
     public KeyStroke getKey(KeyBinding bindName) {
 
-        String s = get(bindName.getKey());
+        String s = get(bindName.getConstant());
 
         if (OS.OS_X) {
             return getKeyForMac(KeyStroke.getKeyStroke(s));
@@ -141,7 +141,7 @@ public KeyStroke getKey(KeyBinding bindName) {
     }
 
     private KeyCombination getKeyCombination(KeyBinding bindName) {
-        String binding = get(bindName.getKey());
+        String binding = get(bindName.getConstant());
         return KeyCombination.valueOf(binding);
     }
 
@@ -191,7 +191,7 @@ private KeyStroke getKeyForMac(KeyStroke ks) {
     }
 
     public List<String> getBindNames() {
-        return bindings.keySet().stream().map(KeyBinding::getKey).collect(Collectors.toList());
+        return bindings.keySet().stream().map(KeyBinding::getConstant).collect(Collectors.toList());
     }
 
     public List<String> getBindings() {

diff --git a/src/main/java/org/jabref/gui/keyboard/KeyBindingViewModel.java b/src/main/java/org/jabref/gui/keyboard/KeyBindingViewModel.java
@@ -128,7 +128,7 @@ public boolean setNewBinding(KeyEvent evt) {
      */
     public void resetToDefault() {
         if (!isCategory()) {
-            String key = getKeyBinding().getKey();
+            String key = getKeyBinding().getConstant();
             keyBindingRepository.resetToDefault(key);
             setBinding(keyBindingRepository.get(key));
         }

diff --git a/src/test/java/org/jabref/gui/keyboard/KeyBindingsDialogViewModelTest.java b/src/test/java/org/jabref/gui/keyboard/KeyBindingsDialogViewModelTest.java
@@ -10,6 +10,7 @@
 import org.junit.Before;
 import org.junit.Test;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -86,7 +87,7 @@ public void testRandomNewKeyKeyBindingInRepository() {
 
         assertTrue(keyBindingRepository.checkKeyCombinationEquality(combination, shortcutKeyEvent));
 
-        assertFalse(keyBindingRepository.checkKeyCombinationEquality(KeyCombination.valueOf(KeyBinding.CLEANUP.getDefaultBinding()), shortcutKeyEvent));
+        assertFalse(keyBindingRepository.checkKeyCombinationEquality(KeyCombination.valueOf(KeyBinding.CLEANUP.getDefaultKeyBinding()), shortcutKeyEvent));
     }
 
     @Test
@@ -133,6 +134,21 @@ public void testSetAllKeyBindingsToDefault() {
         assertFalse(keyBindingRepository.checkKeyCombinationEquality(KeyBinding.ABBREVIATE, shortcutKeyEvent));
     }
 
+    @Test
+    public void testCloseEntryEditorCloseEntryKeybinding() {
+        KeyBindingViewModel viewModel = setKeyBindingViewModel(KeyBinding.CLOSE_ENTRY_EDITOR);
+        model.selectedKeyBindingProperty().set(viewModel);
+        KeyEvent closeEditorEvent = new KeyEvent(KeyEvent.KEY_PRESSED, "", "", KeyCode.ESCAPE, false, false, false, false);
+
+        assertEquals(KeyBinding.CLOSE_ENTRY_EDITOR.getDefaultKeyBinding(), KeyCode.ESCAPE.getName());
+
+        KeyCombination combi = KeyCombination.valueOf(KeyBinding.CLOSE_ENTRY_EDITOR.getDefaultKeyBinding());
+
+        assertTrue(combi.match(closeEditorEvent));
+        assertTrue(keyBindingRepository.checkKeyCombinationEquality(KeyBinding.CLOSE_ENTRY_EDITOR, closeEditorEvent));
+
+    }
+
     @Test
     public void testSetSingleKeyBindingToDefault() {
         KeyBindingViewModel viewModel = setKeyBindingViewModel(KeyBinding.ABBREVIATE);
@@ -153,7 +169,7 @@ public void testSetSingleKeyBindingToDefault() {
     }
 
     private KeyBindingViewModel setKeyBindingViewModel(KeyBinding binding) {
-        KeyBindingViewModel bindViewModel = new KeyBindingViewModel(keyBindingRepository, binding, binding.getDefaultBinding());
+        KeyBindingViewModel bindViewModel = new KeyBindingViewModel(keyBindingRepository, binding, binding.getDefaultKeyBinding());
         model.selectedKeyBindingProperty().set(bindViewModel);
         return bindViewModel;
     }