From db96f88659882e188cfada382bba834183779731 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Wed, 18 Sep 2019 12:02:36 +0200 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + src/main/java/org/jabref/gui/BasePanel.java | 6 +- src/main/java/org/jabref/gui/JabRefFrame.java | 21 ++++--- .../jabref/gui/search/GlobalSearchBar.java | 62 +++++++++---------- 4 files changed, 45 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53505257d73..305433eeb77 100644 --- 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 index 9c2f2ddb6d2..b35a6bdce4d 100644 --- a/src/main/java/org/jabref/gui/BasePanel.java +++ b/src/main/java/org/jabref/gui/BasePanel.java @@ -1051,11 +1051,9 @@ public Optional 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 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 index ae00ada1df4..57190ff1072 100644 --- 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 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 index 7f3b6dec832..6938411b4ad 100644 --- 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 AutoCompletePopup getPopup(AutoCompletionBinding 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);