Clarify the duplicates resolver dialog actions (#9140)

* Don't show the info button when the authors field content is identical * Use enhanced switch expression * Improve duplicate resolver dialog actions - Renamed the actions. - Changed their layout; some actions on the left and some on the right. - Commented out the help action for now. * Move 'Keep old entry' action to the left * do something * Remove DuplicateResolverType#INSPECTION - This enum was used by to decide what buttons to add to the duplicate resolver dialog in ImportEntriesDialog. However, both ImportHandler and ImportEntriesDialog have the same buttons. In addition, they share logic too. We should consider merging them at some point. * i18n * Layout the 'Automatically remove exact duplicates' button in the button bar * Add help button * Update CHANGELOG.md
JabRef · Sep 14, 2022 · 6a4a4f4 · 6a4a4f4
1 parent 9604f7d
commit 6a4a4f4
Show file tree

Hide file tree

Showing 7 changed files with 56 additions and 56 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -32,6 +32,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - The fallback directory of the file folder now is the general file directory. In case there was a directory configured for a library and this directory was not found, JabRef placed the PDF next to the .bib file and not into the general file directory.
 - The global default directory for storing PDFs is now the subdirectory "JabRef" in the user's home.
 - We reworked the Define study parameters dialog. [#9123](https://github.com/JabRef/jabref/pull/9123)
+- We simplified the actions to fast-resolve duplicates to 'Keep Left', 'Keep Right', 'Keep Both' and 'Keep Merged'. [#9056](https://github.com/JabRef/jabref/issues/9056)
 
 
 ### Fixed

diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java
@@ -820,7 +820,7 @@ private MenuBar createMenu() {
         );
 
         quality.getItems().addAll(
-                factory.createMenuItem(StandardActions.FIND_DUPLICATES, new DuplicateSearch(this, dialogService, stateManager)),
+                factory.createMenuItem(StandardActions.FIND_DUPLICATES, new DuplicateSearch(this, dialogService, stateManager, prefs)),
                 factory.createMenuItem(StandardActions.MERGE_ENTRIES, new MergeEntriesAction(dialogService, stateManager)),
                 factory.createMenuItem(StandardActions.CHECK_INTEGRITY, new IntegrityCheckAction(this, stateManager, Globals.TASK_EXECUTOR)),
                 factory.createMenuItem(StandardActions.CLEANUP_ENTRIES, new CleanupAction(this, this.prefs, dialogService, stateManager)),

diff --git a/src/main/java/org/jabref/gui/duplicationFinder/DuplicateResolverDialog.java b/src/main/java/org/jabref/gui/duplicationFinder/DuplicateResolverDialog.java
@@ -8,6 +8,8 @@
 
 import org.jabref.gui.DialogService;
 import org.jabref.gui.StateManager;
+import org.jabref.gui.actions.ActionFactory;
+import org.jabref.gui.actions.StandardActions;
 import org.jabref.gui.duplicationFinder.DuplicateResolverDialog.DuplicateResolverResult;
 import org.jabref.gui.help.HelpAction;
 import org.jabref.gui.mergeentries.newmergedialog.ThreeWayMergeView;
@@ -17,6 +19,7 @@
 import org.jabref.logic.l10n.Localization;
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
+import org.jabref.preferences.PreferencesService;
 
 public class DuplicateResolverDialog extends BaseDialog<DuplicateResolverResult> {
 
@@ -26,7 +29,6 @@ public class DuplicateResolverDialog extends BaseDialog<DuplicateResolverResult>
     public enum DuplicateResolverType {
         DUPLICATE_SEARCH,
         IMPORT_CHECK,
-        INSPECTION,
         DUPLICATE_SEARCH_WITH_EXACT
     }
 
@@ -41,65 +43,63 @@ public enum DuplicateResolverResult {
 
     private ThreeWayMergeView threeWayMerge;
     private final DialogService dialogService;
+    private final ActionFactory actionFactory;
 
-    public DuplicateResolverDialog(BibEntry one, BibEntry two, DuplicateResolverType type, BibDatabaseContext database, StateManager stateManager, DialogService dialogService) {
+    public DuplicateResolverDialog(BibEntry one, BibEntry two, DuplicateResolverType type, BibDatabaseContext database, StateManager stateManager, DialogService dialogService, PreferencesService prefs) {
         this.setTitle(Localization.lang("Possible duplicate entries"));
         this.database = database;
         this.stateManager = stateManager;
         this.dialogService = dialogService;
+        this.actionFactory = new ActionFactory(prefs.getKeyBindingRepository());
         init(one, two, type);
     }
 
     private void init(BibEntry one, BibEntry two, DuplicateResolverType type) {
-        HelpAction helpCommand = new HelpAction(HelpFile.FIND_DUPLICATES, dialogService);
-        ButtonType help = new ButtonType(Localization.lang("Help"), ButtonData.HELP);
-
         ButtonType cancel = ButtonType.CANCEL;
-        ButtonType merge = new ButtonType(Localization.lang("Keep merged entry only"), ButtonData.APPLY);
+        ButtonType merge = new ButtonType(Localization.lang("Keep merged"), ButtonData.OK_DONE);
 
-        ButtonBar options = new ButtonBar();
         ButtonType both;
         ButtonType second;
         ButtonType first;
-        ButtonType removeExact = new ButtonType(Localization.lang("Automatically remove exact duplicates"), ButtonData.APPLY);
+        ButtonType removeExact = new ButtonType(Localization.lang("Automatically remove exact duplicates"), ButtonData.LEFT);
         boolean removeExactVisible = false;
 
         switch (type) {
-            case DUPLICATE_SEARCH:
-                first = new ButtonType(Localization.lang("Keep left"), ButtonData.APPLY);
-                second = new ButtonType(Localization.lang("Keep right"), ButtonData.APPLY);
-                both = new ButtonType(Localization.lang("Keep both"), ButtonData.APPLY);
+            case DUPLICATE_SEARCH -> {
+                first = new ButtonType(Localization.lang("Keep left"), ButtonData.LEFT);
+                second = new ButtonType(Localization.lang("Keep right"), ButtonData.LEFT);
+                both = new ButtonType(Localization.lang("Keep both"), ButtonData.LEFT);
                 threeWayMerge = new ThreeWayMergeView(one, two);
-                break;
-            case INSPECTION:
-                first = new ButtonType(Localization.lang("Remove old entry"), ButtonData.APPLY);
-                second = new ButtonType(Localization.lang("Remove entry from import"), ButtonData.APPLY);
-                both = new ButtonType(Localization.lang("Keep both"), ButtonData.APPLY);
-                threeWayMerge = new ThreeWayMergeView(one, two, Localization.lang("Old entry"),
-                                                      Localization.lang("From import"));
-                break;
-            case DUPLICATE_SEARCH_WITH_EXACT:
-                first = new ButtonType(Localization.lang("Keep left"), ButtonData.APPLY);
-                second = new ButtonType(Localization.lang("Keep right"), ButtonData.APPLY);
-                both = new ButtonType(Localization.lang("Keep both"), ButtonData.APPLY);
-
+            }
+            case DUPLICATE_SEARCH_WITH_EXACT -> {
+                first = new ButtonType(Localization.lang("Keep left"), ButtonData.LEFT);
+                second = new ButtonType(Localization.lang("Keep right"), ButtonData.LEFT);
+                both = new ButtonType(Localization.lang("Keep both"), ButtonData.LEFT);
                 removeExactVisible = true;
-
                 threeWayMerge = new ThreeWayMergeView(one, two);
-                break;
-            default:
-                first = new ButtonType(Localization.lang("Import and remove old entry"), ButtonData.APPLY);
-                second = new ButtonType(Localization.lang("Do not import entry"), ButtonData.APPLY);
-                both = new ButtonType(Localization.lang("Import and keep old entry"), ButtonData.APPLY);
+            }
+            case IMPORT_CHECK -> {
+                first = new ButtonType(Localization.lang("Keep old entry"), ButtonData.LEFT);
+                second = new ButtonType(Localization.lang("Keep from import"), ButtonData.LEFT);
+                both = new ButtonType(Localization.lang("Keep both"), ButtonData.LEFT);
                 threeWayMerge = new ThreeWayMergeView(one, two, Localization.lang("Old entry"),
-                                                      Localization.lang("From import"));
-                break;
+                        Localization.lang("From import"));
+            }
+            default -> throw new IllegalStateException("Switch expression should be exhaustive");
         }
+
+        this.getDialogPane().getButtonTypes().addAll(first, second, both, merge, cancel);
+        this.getDialogPane().setFocusTraversable(false);
+
         if (removeExactVisible) {
             this.getDialogPane().getButtonTypes().add(removeExact);
-        }
 
-        this.getDialogPane().getButtonTypes().addAll(first, second, both, merge, cancel, help);
+            // This will prevent all dialog buttons from having the same size
+            // Read more: https://stackoverflow.com/questions/45866249/javafx-8-alert-different-button-sizes
+            getDialogPane().getButtonTypes().stream()
+                           .map(getDialogPane()::lookupButton)
+                           .forEach(btn-> ButtonBar.setButtonUniformSize(btn, false));
+        }
 
         // Retrieves the previous window state and sets the new dialog window size and position to match it
         DialogWindowState state = stateManager.getDialogWindowState(getClass().getSimpleName());
@@ -110,7 +110,6 @@ private void init(BibEntry one, BibEntry two, DuplicateResolverType type) {
         }
 
         BorderPane borderPane = new BorderPane(threeWayMerge);
-        borderPane.setBottom(options);
 
         this.setResultConverter(button -> {
             // Updates the window state on button press
@@ -130,9 +129,11 @@ private void init(BibEntry one, BibEntry two, DuplicateResolverType type) {
             return null;
         });
 
+        HelpAction helpCommand = new HelpAction(HelpFile.FIND_DUPLICATES, dialogService);
+        Button helpButton = actionFactory.createIconButton(StandardActions.HELP, helpCommand);
+        borderPane.setRight(helpButton);
+
         getDialogPane().setContent(borderPane);
-        Button helpButton = (Button) this.getDialogPane().lookupButton(help);
-        helpButton.setOnAction(evt -> helpCommand.execute());
     }
 
     public BibEntry getMergedEntry() {

diff --git a/src/main/java/org/jabref/gui/duplicationFinder/DuplicateSearch.java b/src/main/java/org/jabref/gui/duplicationFinder/DuplicateSearch.java
@@ -34,6 +34,7 @@
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.database.BibDatabaseMode;
 import org.jabref.model.entry.BibEntry;
+import org.jabref.preferences.PreferencesService;
 
 import static org.jabref.gui.actions.ActionHelper.needsDatabase;
 
@@ -51,10 +52,13 @@ public class DuplicateSearch extends SimpleCommand {
     private final DialogService dialogService;
     private final StateManager stateManager;
 
-    public DuplicateSearch(JabRefFrame frame, DialogService dialogService, StateManager stateManager) {
+    private final PreferencesService prefs;
+
+    public DuplicateSearch(JabRefFrame frame, DialogService dialogService, StateManager stateManager, PreferencesService prefs) {
         this.frame = frame;
         this.dialogService = dialogService;
         this.stateManager = stateManager;
+        this.prefs = prefs;
 
         this.executable.bind(needsDatabase(stateManager));
     }
@@ -142,7 +146,7 @@ private DuplicateSearchResult verifyDuplicates() {
     }
 
     private void askResolveStrategy(DuplicateSearchResult result, BibEntry first, BibEntry second, DuplicateResolverType resolverType) {
-        DuplicateResolverDialog dialog = new DuplicateResolverDialog(first, second, resolverType, frame.getCurrentLibraryTab().getBibDatabaseContext(), stateManager, dialogService);
+        DuplicateResolverDialog dialog = new DuplicateResolverDialog(first, second, resolverType, frame.getCurrentLibraryTab().getBibDatabaseContext(), stateManager, dialogService, prefs);
 
         dialog.titleProperty().bind(Bindings.concat(dialog.getTitle()).concat(" (").concat(duplicateProgress.getValue()).concat("/").concat(duplicateTotal).concat(")"));
 

diff --git a/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java b/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java
@@ -195,9 +195,9 @@ public void importEntryWithDuplicateCheck(BibDatabaseContext bibDatabaseContext,
 
         Optional<BibEntry> existingDuplicateInLibrary = new DuplicateCheck(Globals.entryTypesManager).containsDuplicate(bibDatabaseContext.getDatabase(), entryToInsert, bibDatabaseContext.getMode());
         if (existingDuplicateInLibrary.isPresent()) {
-            DuplicateResolverDialog dialog = new DuplicateResolverDialog(existingDuplicateInLibrary.get(), entryToInsert, DuplicateResolverDialog.DuplicateResolverType.IMPORT_CHECK, bibDatabaseContext, stateManager, dialogService);
+            DuplicateResolverDialog dialog = new DuplicateResolverDialog(existingDuplicateInLibrary.get(), entryToInsert, DuplicateResolverDialog.DuplicateResolverType.IMPORT_CHECK, bibDatabaseContext, stateManager, dialogService, preferencesService);
             switch (dialogService.showCustomDialogAndWait(dialog).orElse(DuplicateResolverDialog.DuplicateResolverResult.BREAK)) {
-                case KEEP_LEFT:
+                case KEEP_RIGHT:
                     bibDatabaseContext.getDatabase().removeEntry(existingDuplicateInLibrary.get());
                     break;
                 case KEEP_BOTH:
@@ -206,7 +206,7 @@ public void importEntryWithDuplicateCheck(BibDatabaseContext bibDatabaseContext,
                     bibDatabaseContext.getDatabase().removeEntry(existingDuplicateInLibrary.get());
                     entryToInsert = dialog.getMergedEntry();
                     break;
-                case KEEP_RIGHT:
+                case KEEP_LEFT:
                 case AUTOREMOVE_EXACT:
                 case BREAK:
                 default:

diff --git a/src/main/java/org/jabref/gui/importer/ImportEntriesViewModel.java b/src/main/java/org/jabref/gui/importer/ImportEntriesViewModel.java
@@ -197,7 +197,7 @@ public void resolveDuplicate(BibEntry entry) {
         Optional<BibEntry> other = new DuplicateCheck(entryTypesManager).containsDuplicate(databaseContext.getDatabase(), entry, databaseContext.getMode());
         if (other.isPresent()) {
             DuplicateResolverDialog dialog = new DuplicateResolverDialog(other.get(),
-                    entry, DuplicateResolverDialog.DuplicateResolverType.INSPECTION, databaseContext, stateManager, dialogService);
+                    entry, DuplicateResolverDialog.DuplicateResolverType.IMPORT_CHECK, databaseContext, stateManager, dialogService, preferences);
 
             DuplicateResolverDialog.DuplicateResolverResult result = dialogService.showCustomDialogAndWait(dialog)
                                                                                   .orElse(DuplicateResolverDialog.DuplicateResolverResult.BREAK);
@@ -228,7 +228,7 @@ public void resolveDuplicate(BibEntry entry) {
         other = findInternalDuplicate(entry);
         if (other.isPresent()) {
             DuplicateResolverDialog diag = new DuplicateResolverDialog(entry,
-                    other.get(), DuplicateResolverDialog.DuplicateResolverType.DUPLICATE_SEARCH, databaseContext, stateManager, dialogService);
+                    other.get(), DuplicateResolverDialog.DuplicateResolverType.DUPLICATE_SEARCH, databaseContext, stateManager, dialogService, preferences);
 
             DuplicateResolverDialog.DuplicateResolverResult answer = dialogService.showCustomDialogAndWait(diag)
                                                                                   .orElse(DuplicateResolverDialog.DuplicateResolverResult.BREAK);

diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties
@@ -260,7 +260,6 @@ Display\ version=Display version
 
 Do\ not\ abbreviate\ names=Do not abbreviate names
 
-Do\ not\ import\ entry=Do not import entry
 
 Do\ not\ open\ any\ files\ at\ startup=Do not open any files at startup
 
@@ -431,10 +430,6 @@ Ignore=Ignore
 
 Import=Import
 
-Import\ and\ keep\ old\ entry=Import and keep old entry
-
-Import\ and\ remove\ old\ entry=Import and remove old entry
-
 Import\ entries=Import entries
 Import\ file=Import file
 
@@ -688,8 +683,6 @@ Remove\ subgroups=Remove subgroups
 
 Remove\ all\ subgroups\ of\ "%0"?=Remove all subgroups of "%0"?
 
-Remove\ entry\ from\ import=Remove entry from import
-
 Remove\ selected\ entries\ from\ this\ group=Remove selected entries from this group
 
 Remove\ group=Remove group
@@ -716,8 +709,6 @@ Removed\ all\ selected\ groups\ and\ their\ subgroups.=Removed all selected grou
 
 Remove\ link=Remove link
 
-Remove\ old\ entry=Remove old entry
-
 Remove\ string\ %0=Remove string %0
 
 Removed\ group\ "%0".=Removed group "%0".
@@ -1346,7 +1337,7 @@ Warning\:\ %0\ out\ of\ %1\ entries\ have\ undefined\ citation\ key.=Warning: %0
 Warning\:\ %0\ out\ of\ %1\ entries\ have\ undefined\ DOIs.=Warning: %0 out of %1 entries have undefined DOIs.
 Really\ delete\ the\ selected\ entry?=Really delete the selected entry?
 Really\ delete\ the\ %0\ selected\ entries?=Really delete the %0 selected entries?
-Keep\ merged\ entry\ only=Keep merged entry only
+
 Keep\ left=Keep left
 Keep\ right=Keep right
 Old\ entry=Old entry
@@ -2514,3 +2505,6 @@ plain\ text=plain text
 
 The\ %0s\ are\ the\ same.\ However,\ the\ order\ of\ field\ content\ differs=The %0s are the same. However, the order of field content differs
 
+Keep\ from\ import=Keep from import
+Keep\ merged=Keep merged
+Keep\ old\ entry=Keep old entry