Fix problem with search and switching between libraries (#5326)

* Fix problem with search and switching between libraries Fixes #4846 by using bindings instead of listeners. * Remove wrong javadoc
JabRef · Sep 18, 2019 · db96f88 · db96f88
1 parent 1601770
commit db96f88
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 45 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 - The group panel is now properly updated when switching between libraries (or when closing/opening one). [#3142](https://github.com/JabRef/jabref/issues/3142)
 - We fixed an error where the number of matched entries shown in the group pane was not updated correctly. [#4441](https://github.com/JabRef/jabref/issues/4441)
 - We fixed an error mentioning "javafx.controls/com.sun.javafx.scene.control" that was thrown when interacting with the toolbar.
+- We fixed an error where a cleared search was restored after switching libraries. [#4846](https://github.com/JabRef/jabref/issues/4846) 
 
 ### Removed
 

diff --git a/src/main/java/org/jabref/gui/BasePanel.java b/src/main/java/org/jabref/gui/BasePanel.java
@@ -1051,11 +1051,9 @@ public Optional<SearchQuery> getCurrentSearchQuery() {
 
     /**
      * Set the query the user currently searches while this basepanel is active
-     *
-     * @param currentSearchQuery can be null
      */
-    public void setCurrentSearchQuery(SearchQuery currentSearchQuery) {
-        this.currentSearchQuery = Optional.ofNullable(currentSearchQuery);
+    public void setCurrentSearchQuery(Optional<SearchQuery> currentSearchQuery) {
+        this.currentSearchQuery = currentSearchQuery;
     }
 
     public CitationStyleCache getCitationStyleCache() {

diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java
@@ -109,7 +109,6 @@
 import org.jabref.logic.importer.ParserResult;
 import org.jabref.logic.importer.WebFetchers;
 import org.jabref.logic.l10n.Localization;
-import org.jabref.logic.search.SearchQuery;
 import org.jabref.logic.undo.AddUndoableActionEvent;
 import org.jabref.logic.undo.UndoChangeEvent;
 import org.jabref.logic.undo.UndoRedoEvent;
@@ -141,7 +140,7 @@ public class JabRefFrame extends BorderPane {
 
     private final SplitPane splitPane = new SplitPane();
     private final JabRefPreferences prefs = Globals.prefs;
-    private final GlobalSearchBar globalSearchBar = new GlobalSearchBar(this);
+    private final GlobalSearchBar globalSearchBar = new GlobalSearchBar(this, Globals.stateManager);
 
     private final ProgressBar progressBar = new ProgressBar();
     private final FileHistoryMenu fileHistory = new FileHistoryMenu(prefs, this);
@@ -577,6 +576,15 @@ public void init() {
         stateManager.activeDatabaseProperty().bind(
                 EasyBind.map(tabbedPane.getSelectionModel().selectedItemProperty(),
                         tab -> Optional.ofNullable(tab).map(JabRefFrame::getBasePanel).map(BasePanel::getBibDatabaseContext)));
+
+        // Subscribe to the search
+        EasyBind.subscribe(stateManager.activeSearchQueryProperty(),
+                query -> {
+                    if (getCurrentBasePanel() != null) {
+                        getCurrentBasePanel().setCurrentSearchQuery(query);
+                    }
+                });
+
         /*
          * The following state listener makes sure focus is registered with the
          * correct database when the user switches tabs. Without this,
@@ -592,13 +600,8 @@ public void init() {
             // Poor-mans binding to global state
             stateManager.setSelectedEntries(newBasePanel.getSelectedEntries());
 
-            // Update search query
-            String content = "";
-            Optional<SearchQuery> currentSearchQuery = newBasePanel.getCurrentSearchQuery();
-            if (currentSearchQuery.isPresent()) {
-                content = currentSearchQuery.get().getQuery();
-            }
-            globalSearchBar.setSearchTerm(content);
+            // Update active search query when switching between databases
+            stateManager.activeSearchQueryProperty().set(newBasePanel.getCurrentSearchQuery());
 
             // groupSidePane.getToggleCommand().setSelected(sidePaneManager.isComponentVisible(GroupSidePane.class));
             //previewToggle.setSelected(Globals.prefs.getPreviewPreferences().isPreviewPanelEnabled());

diff --git a/src/main/java/org/jabref/gui/search/GlobalSearchBar.java b/src/main/java/org/jabref/gui/search/GlobalSearchBar.java
@@ -35,6 +35,7 @@
 import org.jabref.Globals;
 import org.jabref.gui.BasePanel;
 import org.jabref.gui.JabRefFrame;
+import org.jabref.gui.StateManager;
 import org.jabref.gui.autocompleter.AppendPersonNamesStrategy;
 import org.jabref.gui.autocompleter.AutoCompleteFirstNameMode;
 import org.jabref.gui.autocompleter.AutoCompleteSuggestionProvider;
@@ -45,6 +46,7 @@
 import org.jabref.gui.keyboard.KeyBindingRepository;
 import org.jabref.gui.maintable.MainTable;
 import org.jabref.gui.search.rules.describer.SearchDescribers;
+import org.jabref.gui.util.BindingsHelper;
 import org.jabref.gui.util.DefaultTaskExecutor;
 import org.jabref.gui.util.TooltipTextUtil;
 import org.jabref.logic.l10n.Localization;
@@ -77,11 +79,14 @@ public class GlobalSearchBar extends HBox {
     private final Button searchModeButton = new Button();
     private final Label currentResults = new Label("");
     private final Tooltip tooltip = new Tooltip();
+    private final StateManager stateManager;
     private SearchDisplayMode searchDisplayMode;
 
-    public GlobalSearchBar(JabRefFrame frame) {
+    public GlobalSearchBar(JabRefFrame frame, StateManager stateManager) {
         super();
         this.frame = Objects.requireNonNull(frame);
+        this.stateManager = stateManager;
+
         SearchPreferences searchPreferences = new SearchPreferences(Globals.prefs);
         searchDisplayMode = searchPreferences.getSearchMode();
 
@@ -99,7 +104,7 @@ public GlobalSearchBar(JabRefFrame frame) {
             if (keyBinding.isPresent()) {
                 if (keyBinding.get().equals(KeyBinding.CLOSE)) {
                     // Clear search and select first entry, if available
-                    clearSearch();
+                    searchField.setText("");
                     frame.getCurrentBasePanel().getMainTable().getSelectionModel().selectFirst();
                     event.consume();
                 }
@@ -132,10 +137,7 @@ public GlobalSearchBar(JabRefFrame frame) {
         searchField.setMaxWidth(initialSize);
         HBox.setHgrow(searchField, Priority.ALWAYS);
 
-        Timer searchTask = FxTimer.create(java.time.Duration.ofMillis(SEARCH_DELAY), () -> {
-            LOGGER.debug("Run search " + searchField.getText());
-            performSearch();
-        });
+        Timer searchTask = FxTimer.create(java.time.Duration.ofMillis(SEARCH_DELAY), this::performSearch);
         searchField.textProperty().addListener((observable, oldValue, newValue) -> searchTask.restart());
 
         EasyBind.subscribe(searchField.focusedProperty(), isFocused -> {
@@ -156,9 +158,19 @@ public GlobalSearchBar(JabRefFrame frame) {
 
         this.setAlignment(Pos.CENTER_LEFT);
 
-        EasyBind.subscribe(Globals.stateManager.activeSearchQueryProperty(), searchQuery -> {
+        BindingsHelper.bindBidirectional(
+                stateManager.activeSearchQueryProperty(),
+                searchField.textProperty(),
+                searchTerm -> {
+                    performSearch();
+                },
+                query -> setSearchTerm(query.map(SearchQuery::getQuery).orElse(""))
+        );
+
+        EasyBind.subscribe(this.stateManager.activeSearchQueryProperty(), searchQuery -> {
+
             searchQuery.ifPresent(query -> {
-                updateResults(Globals.stateManager.getSearchResultSize().intValue(), SearchDescribers.getSearchDescriberFor(query).getDescription(),
+                updateResults(this.stateManager.getSearchResultSize().intValue(), SearchDescribers.getSearchDescriberFor(query).getDescription(),
                               query.isGrammarBasedSearch());
             });
         });
@@ -180,7 +192,7 @@ private void updateSearchModeButtonText() {
     public void endSearch() {
         BasePanel currentBasePanel = frame.getCurrentBasePanel();
         if (currentBasePanel != null) {
-            clearSearch();
+            searchField.setText("");
             MainTable mainTable = frame.getCurrentBasePanel().getMainTable();
             mainTable.requestFocus();
         }
@@ -196,38 +208,30 @@ public void focus() {
         searchField.selectAll();
     }
 
-    private void clearSearch() {
-        currentResults.setText("");
-        searchField.setText("");
-        setHintTooltip(null);
-        Globals.stateManager.clearSearchQuery();
-    }
-
     public void performSearch() {
-        BasePanel currentBasePanel = frame.getCurrentBasePanel();
-        if (currentBasePanel == null) {
-            return;
-        }
+        LOGGER.debug("Run search " + searchField.getText());
+
         // An empty search field should cause the search to be cleared.
         if (searchField.getText().isEmpty()) {
-            clearSearch();
+            currentResults.setText("");
+            setHintTooltip(null);
+            stateManager.clearSearchQuery();
             return;
         }
 
-        SearchQuery searchQuery = getSearchQuery();
+        SearchQuery searchQuery = new SearchQuery(this.searchField.getText(), this.caseSensitive.isSelected(), this.regularExp.isSelected());
         if (!searchQuery.isValid()) {
             informUserAboutInvalidSearchQuery();
             return;
         }
 
-        Globals.stateManager.setSearchQuery(searchQuery);
-
+        stateManager.setSearchQuery(searchQuery);
     }
 
     private void informUserAboutInvalidSearchQuery() {
         searchField.pseudoClassStateChanged(CLASS_NO_RESULTS, true);
 
-        Globals.stateManager.clearSearchQuery();
+        stateManager.clearSearchQuery();
 
         String illegalSearch = Localization.lang("Search failed: illegal search expression");
         currentResults.setText(illegalSearch);
@@ -260,13 +264,7 @@ private <T> AutoCompletePopup<T> getPopup(AutoCompletionBinding<T> autoCompletio
         }
     }
 
-    private SearchQuery getSearchQuery() {
-        SearchQuery searchQuery = new SearchQuery(this.searchField.getText(), this.caseSensitive.isSelected(), this.regularExp.isSelected());
-        this.frame.getCurrentBasePanel().setCurrentSearchQuery(searchQuery);
-        return searchQuery;
-    }
-
-    public void updateResults(int matched, TextFlow description, boolean grammarBasedSearch) {
+    private void updateResults(int matched, TextFlow description, boolean grammarBasedSearch) {
         if (matched == 0) {
             currentResults.setText(Localization.lang("No results found."));
             searchField.pseudoClassStateChanged(CLASS_NO_RESULTS, true);