Skip to content

Commit

Permalink
Fix for issue #3381 (#3443)
Browse files Browse the repository at this point in the history
* the SearchDescribers now return a TextFlow

* moved the search.rules.describer package from logic to gui package
  because the SearchDescriber now creates TextFlow Elements

* moved the tests accordingly

* modified creation of Tooltip in GlobalSearchBar to display the
  TextFlow created by the SearchDescribers

* SearchQuery now stores a TextFlow as description

* Added a stub for compatibility in GroupDialog because it uses the
  SearchQuery to get the description and needs an HTML string for
  its Swing Tooltip

* Styled the SearchBar Tooltip for readability

It uses an own style class to not change any other styled elements.
Styling was choosen to look like the other (swing) tooltips.

* refactor for better readability

* removed description member of SearchQuery class

because the description of a SearchQuery is a gui element, it is not
stored any more in this class. Everywhere the description is needed it
is generated

* add method to create HTML string from javafx Text

* create HTML string from description for compat.

* fixed formatting for style

* created tests for TextUtil class

* modified the SearchDescriber Implementations tests

now they work properly again and check the created Texts inside
the description TextFlow

* split up testcases into single methods

* using old localized strings again

these old localized strings containing html tags are broken up and
converted to Text objects to use in javafx TextFlow

* added and modified tests

* added changelog entry

* changed syntax for getting SearchDescriber

now a whole SearchQuery object is sufficient to get a corresponding
SearchDescriber

* refactored for better style

* javafx tooltips styled per css

also refactored the GlobalSearchBar tooltip to use this new styling

* modified and restructured tests

It was necessary to modify the tests of the SearchDescriber
Implementations and the TooltipTextUtil class because of the new css
styling for tooltips. Also refactored for better style
  • Loading branch information
jupf authored and tobiasdiez committed Nov 27, 2017
1 parent 9e2c010 commit e944c7a
Show file tree
Hide file tree
Showing 20 changed files with 737 additions and 297 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
- We fixed an issue where errors in citation styles triggered an exception when opening the preferences dialog [#3389](https://github.com/JabRef/jabref/issues/3389)
- We fixed an issue where entries were displayed twice after insertion into a shared database. [#3164](https://github.com/JabRef/jabref/issues/3164)
- We fixed an issue where special fields (such as `printed`) could not be cleared when syncing special fields via the keywords [#3432](https://github.com/JabRef/jabref/issues/3432)

- We fixed an issue where the tooltip of the global search bar showed html tags instead of formatting the text [#3381](https://github.com/JabRef/jabref/issues/3381)

### Removed

Expand Down
23 changes: 21 additions & 2 deletions src/main/java/org/jabref/gui/Main.css
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,27 @@
}

.tooltip {
-fx-background-color: #757575; /* For some reason it does not work if we use -fx-dark-background here */
-fx-effect:none;
-fx-background-color: #616161;
-fx-opacity: 95%;
-fx-text-fill: rgba(255, 255, 255, 0.9);
-fx-font-size: 1em;
}

.tooltip > TextFlow > Text {
-fx-fill: rgba(255, 255, 255, 0.9);
-fx-font-size: 1em;
}

.tooltip > TextFlow > .tooltip-text-bold {
-fx-font-weight: bold;
}

.tooltip > TextFlow > .tooltip-text-italic {
-fx-font-style: italic;
}

.tooltip > TextFlow > .tooltip-text-monospaced {
-fx-font-family: monospace;
}

.flatButton {
Expand Down
19 changes: 17 additions & 2 deletions src/main/java/org/jabref/gui/groups/GroupDialog.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@
import javax.swing.SwingConstants;
import javax.swing.event.CaretListener;

import javafx.scene.Node;
import javafx.scene.paint.Color;
import javafx.scene.text.Text;
import javafx.scene.text.TextFlow;

import org.jabref.Globals;
import org.jabref.JabRefGUI;
Expand All @@ -36,6 +39,8 @@
import org.jabref.gui.JabRefFrame;
import org.jabref.gui.fieldeditors.TextField;
import org.jabref.gui.keyboard.KeyBinding;
import org.jabref.gui.search.rules.describer.SearchDescribers;
import org.jabref.gui.util.TooltipTextUtil;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.search.SearchQuery;
import org.jabref.model.entry.FieldName;
Expand Down Expand Up @@ -325,7 +330,7 @@ public void actionPerformed(ActionEvent e) {
builderAll.getPanel().getActionMap().put("close", cancelAction);

okButton.addActionListener(e -> {
isOkPressed = true;
isOkPressed = true;
try {
String groupName = nameField.getText().trim();
if (explicitRadioButton.isSelected()) {
Expand Down Expand Up @@ -566,7 +571,8 @@ private void updateComponents() {
s1 = searchGroupSearchExpression.getText().trim();
okEnabled = okEnabled & !s1.isEmpty();
if (okEnabled) {
setDescription(new SearchQuery(s1, isCaseSensitive(), isRegex()).getDescription());
setDescription(fromTextFlowToHTMLString(SearchDescribers.getSearchDescriberFor(
new SearchQuery(s1, isCaseSensitive(), isRegex())).getDescription()));

if (isRegex()) {
try {
Expand All @@ -591,6 +597,15 @@ private void updateComponents() {
okButton.setEnabled(okEnabled);
}

private String fromTextFlowToHTMLString(TextFlow textFlow) {
StringBuilder htmlStringBuilder = new StringBuilder();
for (Node node : textFlow.getChildren()) {
if (node instanceof Text)
htmlStringBuilder.append(TooltipTextUtil.textToHTMLString((Text) node));
}
return htmlStringBuilder.toString();
}

private boolean isRegex() {
return searchGroupRegExp.isSelected();
}
Expand Down
11 changes: 9 additions & 2 deletions src/main/java/org/jabref/gui/search/GlobalSearchBar.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@
import javafx.css.PseudoClass;
import javafx.embed.swing.JFXPanel;
import javafx.scene.Scene;
import javafx.scene.control.ContentDisplay;
import javafx.scene.control.TextField;
import javafx.scene.control.Tooltip;
import javafx.scene.text.TextFlow;

import org.jabref.Globals;
import org.jabref.gui.BasePanel;
Expand Down Expand Up @@ -191,6 +193,7 @@ public void actionPerformed(ActionEvent e) {
container = OS.LINUX ? new CustomJFXPanel() : new JFXPanel();
DefaultTaskExecutor.runInJavaFXThread(() -> {
container.setScene(new Scene(searchField));
container.getScene().getStylesheets().add(GlobalSearchBar.class.getResource("../Main.css").toExternalForm());
container.addKeyListener(new SearchKeyAdapter());

});
Expand Down Expand Up @@ -385,15 +388,19 @@ private SearchQuery getSearchQuery() {
return searchQuery;
}

public void updateResults(int matched, String description, boolean grammarBasedSearch) {
public void updateResults(int matched, TextFlow description, boolean grammarBasedSearch) {
if (matched == 0) {
currentResults.setText(Localization.lang("No results found."));
searchField.pseudoClassStateChanged(CLASS_NO_RESULTS, true);
} else {
currentResults.setText(Localization.lang("Found %0 results.", String.valueOf(matched)));
searchField.pseudoClassStateChanged(CLASS_RESULTS_FOUND, true);
}
DefaultTaskExecutor.runInJavaFXThread(() -> searchField.setTooltip(new Tooltip(description)));
Tooltip tooltip = new Tooltip();
tooltip.setContentDisplay(ContentDisplay.GRAPHIC_ONLY);
tooltip.setGraphic(description);
tooltip.setMaxHeight(10);
DefaultTaskExecutor.runInJavaFXThread(() -> searchField.setTooltip(tooltip));
openCurrentResultsInDialog.setEnabled(true);
}

Expand Down
5 changes: 4 additions & 1 deletion src/main/java/org/jabref/gui/search/SearchWorker.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.jabref.gui.BasePanel;
import org.jabref.gui.BasePanelMode;
import org.jabref.gui.maintable.MainTableDataModel;
import org.jabref.gui.search.rules.describer.SearchDescribers;
import org.jabref.logic.search.SearchQuery;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.entry.BibEntry;
Expand Down Expand Up @@ -108,7 +109,9 @@ private void updateUIWithSearchResult(List<BibEntry> matchedEntries) {
}
}

globalSearchBar.updateResults(matchedEntries.size(), searchQuery.getDescription(), searchQuery.isGrammarBasedSearch());
globalSearchBar.updateResults(matchedEntries.size(),
SearchDescribers.getSearchDescriberFor(searchQuery).getDescription(),
searchQuery.isGrammarBasedSearch());
globalSearchBar.getSearchQueryHighlightObservable().fireSearchlistenerEvent(searchQuery);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package org.jabref.gui.search.rules.describer;

import java.util.List;

import javafx.scene.text.Text;
import javafx.scene.text.TextFlow;

import org.jabref.gui.util.TooltipTextUtil;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.search.rules.SentenceAnalyzer;

public class ContainsAndRegexBasedSearchRuleDescriber implements SearchDescriber {

private final boolean regExp;
private final boolean caseSensitive;
private final String query;

public ContainsAndRegexBasedSearchRuleDescriber(boolean caseSensitive, boolean regExp, String query) {
this.caseSensitive = caseSensitive;
this.regExp = regExp;
this.query = query;
}

@Override
public TextFlow getDescription() {
List<String> words = new SentenceAnalyzer(query).getWords();
String firstWord = words.isEmpty() ? "" : words.get(0);

String temp = regExp ? Localization.lang(
"This search contains entries in which any field contains the regular expression <b>%0</b>")
: Localization.lang("This search contains entries in which any field contains the term <b>%0</b>");
List<Text> textList = TooltipTextUtil.formatToTexts(temp, new TooltipTextUtil.TextReplacement("<b>%0</b>", firstWord, TooltipTextUtil.TextType.BOLD));

if (words.size() > 1) {
List<String> unprocessedWords = words.subList(1, words.size());
for (String word : unprocessedWords) {
textList.add(TooltipTextUtil.createText(String.format(" %s ", Localization.lang("and")), TooltipTextUtil.TextType.NORMAL));
textList.add(TooltipTextUtil.createText(word, TooltipTextUtil.TextType.BOLD));
}
}

String genericDescription = "\n\n" + Localization.lang("Hint: To search specific fields only, enter for example:<p><tt>author=smith and title=electrical</tt>");
genericDescription = genericDescription.replace("<p>", "\n");
List<Text> genericDescriptionTexts = TooltipTextUtil.formatToTexts(genericDescription, new TooltipTextUtil.TextReplacement("<tt>author=smith and title=electrical</tt>", "author=smith and title=electrical", TooltipTextUtil.TextType.MONOSPACED));
textList.add(getCaseSensitiveDescription());
textList.addAll(genericDescriptionTexts);

TextFlow searchDescription = new TextFlow();
searchDescription.getChildren().setAll(textList);
return searchDescription;
}

private Text getCaseSensitiveDescription() {
if (caseSensitive) {
return TooltipTextUtil.createText(String.format(" (%s). ", Localization.lang("case sensitive")), TooltipTextUtil.TextType.NORMAL);
} else {
return TooltipTextUtil.createText(String.format(" (%s). ", Localization.lang("case insensitive")), TooltipTextUtil.TextType.NORMAL);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
package org.jabref.gui.search.rules.describer;

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.regex.Pattern;

import javafx.scene.text.Text;
import javafx.scene.text.TextFlow;

import org.jabref.gui.util.TooltipTextUtil;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.search.rules.GrammarBasedSearchRule;
import org.jabref.model.strings.StringUtil;
import org.jabref.search.SearchBaseVisitor;
import org.jabref.search.SearchParser;

import org.antlr.v4.runtime.tree.ParseTree;

public class GrammarBasedSearchRuleDescriber implements SearchDescriber {

private final boolean caseSensitive;
private final boolean regExp;
private final ParseTree parseTree;

public GrammarBasedSearchRuleDescriber(boolean caseSensitive, boolean regExp, ParseTree parseTree) {
this.caseSensitive = caseSensitive;
this.regExp = regExp;
this.parseTree = Objects.requireNonNull(parseTree);
}

@Override
public TextFlow getDescription() {
TextFlow textFlow = new TextFlow();
DescriptionSearchBaseVisitor descriptionSearchBaseVisitor = new DescriptionSearchBaseVisitor();

// describe advanced search expression
textFlow.getChildren().add(TooltipTextUtil.createText(String.format("%s ", Localization.lang("This search contains entries in which")), TooltipTextUtil.TextType.NORMAL));
textFlow.getChildren().addAll(descriptionSearchBaseVisitor.visit(parseTree));
textFlow.getChildren().add(TooltipTextUtil.createText(". ", TooltipTextUtil.TextType.NORMAL));
textFlow.getChildren().add(TooltipTextUtil.createText(caseSensitive ? Localization
.lang("The search is case sensitive.") :
Localization.lang("The search is case insensitive."), TooltipTextUtil.TextType.NORMAL));
return textFlow;
}

private class DescriptionSearchBaseVisitor extends SearchBaseVisitor<List<Text>> {

@Override
public List<Text> visitStart(SearchParser.StartContext context) {
return visit(context.expression());
}

@Override
public List<Text> visitUnaryExpression(SearchParser.UnaryExpressionContext context) {
List<Text> textList = visit(context.expression());
textList.add(0, TooltipTextUtil.createText(Localization.lang("not").concat(" "), TooltipTextUtil.TextType.NORMAL));
return textList;
}

@Override
public List<Text> visitParenExpression(SearchParser.ParenExpressionContext context) {
ArrayList<Text> textList = new ArrayList<>();
textList.add(TooltipTextUtil.createText(String.format("%s", context.expression()), TooltipTextUtil.TextType.NORMAL));
return textList;
}

@Override
public List<Text> visitBinaryExpression(SearchParser.BinaryExpressionContext context) {
List<Text> textList = visit(context.left);
if ("AND".equalsIgnoreCase(context.operator.getText())) {
textList.add(TooltipTextUtil.createText(String.format(" %s ", Localization.lang("and")), TooltipTextUtil.TextType.NORMAL));
} else {
textList.add(TooltipTextUtil.createText(String.format(" %s ", Localization.lang("or")), TooltipTextUtil.TextType.NORMAL));
}
textList.addAll(visit(context.right));
return textList;
}

@Override
public List<Text> visitComparison(SearchParser.ComparisonContext context) {
final List<Text> textList = new ArrayList<>();
final Optional<SearchParser.NameContext> fieldDescriptor = Optional.ofNullable(context.left);
final String value = StringUtil.unquote(context.right.getText(), '"');
if (!fieldDescriptor.isPresent()) {
TextFlow description = new ContainsAndRegexBasedSearchRuleDescriber(caseSensitive, regExp, value).getDescription();
description.getChildren().forEach(it -> textList.add((Text) it));
return textList;
}

final String field = StringUtil.unquote(fieldDescriptor.get().getText(), '"');
final GrammarBasedSearchRule.ComparisonOperator operator = GrammarBasedSearchRule.ComparisonOperator.build(context.operator.getText());

final boolean regExpFieldSpec = !Pattern.matches("\\w+", field);
String temp = regExpFieldSpec ? Localization.lang(
"any field that matches the regular expression <b>%0</b>") : Localization.lang("the field <b>%0</b>");

if (operator == GrammarBasedSearchRule.ComparisonOperator.CONTAINS) {
if (regExp) {
temp = Localization.lang("%0 contains the regular expression <b>%1</b>", temp);
} else {
temp = Localization.lang("%0 contains the term <b>%1</b>", temp);
}
} else if (operator == GrammarBasedSearchRule.ComparisonOperator.EXACT) {
if (regExp) {
temp = Localization.lang("%0 matches the regular expression <b>%1</b>", temp);
} else {
temp = Localization.lang("%0 matches the term <b>%1</b>", temp);
}
} else if (operator == GrammarBasedSearchRule.ComparisonOperator.DOES_NOT_CONTAIN) {
if (regExp) {
temp = Localization.lang("%0 doesn't contain the regular expression <b>%1</b>", temp);
} else {
temp = Localization.lang("%0 doesn't contain the term <b>%1</b>", temp);
}
} else {
throw new IllegalStateException("CANNOT HAPPEN!");
}

List<Text> formattedTexts = TooltipTextUtil.formatToTexts(temp,
new TooltipTextUtil.TextReplacement("<b>%0</b>", field, TooltipTextUtil.TextType.BOLD),
new TooltipTextUtil.TextReplacement("<b>%1</b>", value, TooltipTextUtil.TextType.BOLD));
textList.addAll(formattedTexts);
return textList;
}

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package org.jabref.gui.search.rules.describer;

import javafx.scene.text.TextFlow;

@FunctionalInterface
public interface SearchDescriber {

TextFlow getDescription();

}
Loading

0 comments on commit e944c7a

Please sign in to comment.