From 5f2ffb234d262422138b32efcab0f2adebf58d8b Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com> Date: Fri, 10 Jan 2020 16:04:57 +0100 Subject: [PATCH] Fix for multiple error messages in messed up source editor (#5823) * Fix for unknown gray shade in dark theme * Fix for missing import in module system * Fixed multiple error dialogs with the same message * Refactored FunctionBasedValidators to common interface and harmonised their logic * Fixed storing entry before changing it and validation * Fixes sync between fields and sourceTab * Fixes test --- src/main/java/module-info.java | 1 + src/main/java/org/jabref/gui/Dark.css | 2 +- .../org/jabref/gui/entryeditor/SourceTab.java | 98 ++++++++++--------- .../gui/preferences/AdvancedTabViewModel.java | 11 ++- .../preferences/AppearanceTabViewModel.java | 13 ++- .../gui/preferences/FileTabViewModel.java | 10 +- .../gui/preferences/GeneralTabViewModel.java | 3 +- .../gui/preferences/PreviewTabViewModel.java | 3 +- .../preferences/TableColumnsTabViewModel.java | 10 +- .../preferences/XmpPrivacyTabViewModel.java | 3 +- .../texparser/ParseTexDialogViewModel.java | 3 +- .../jabref/gui/entryeditor/SourceTabTest.java | 2 +- 12 files changed, 85 insertions(+), 74 deletions(-) diff --git a/src/main/java/module-info.java b/src/main/java/module-info.java index 35b47162c42..38158c02c12 100644 --- a/src/main/java/module-info.java +++ b/src/main/java/module-info.java @@ -68,4 +68,5 @@ requires org.jsoup; requires commons.csv; requires io.github.javadiffutils; + requires flowless; } diff --git a/src/main/java/org/jabref/gui/Dark.css b/src/main/java/org/jabref/gui/Dark.css index 148677bfe0a..d87c8ad5e7d 100644 --- a/src/main/java/org/jabref/gui/Dark.css +++ b/src/main/java/org/jabref/gui/Dark.css @@ -60,7 +60,7 @@ } .table-view .groupColumnBackground { - -fx-stroke: -jr-gray-4; + -fx-stroke: -jr-gray-3; } .code-area .lineno { diff --git a/src/main/java/org/jabref/gui/entryeditor/SourceTab.java b/src/main/java/org/jabref/gui/entryeditor/SourceTab.java index c6e08e3f417..ba563be84f4 100644 --- a/src/main/java/org/jabref/gui/entryeditor/SourceTab.java +++ b/src/main/java/org/jabref/gui/entryeditor/SourceTab.java @@ -11,9 +11,9 @@ import javax.swing.undo.UndoManager; +import javafx.beans.InvalidationListener; import javafx.beans.property.ObjectProperty; import javafx.beans.property.SimpleObjectProperty; -import javafx.collections.ListChangeListener; import javafx.geometry.Point2D; import javafx.scene.control.ContextMenu; import javafx.scene.control.Tooltip; @@ -31,7 +31,6 @@ import org.jabref.gui.undo.NamedCompound; import org.jabref.gui.undo.UndoableChangeType; import org.jabref.gui.undo.UndoableFieldChange; -import org.jabref.gui.util.BindingsHelper; import org.jabref.gui.util.DefaultTaskExecutor; import org.jabref.logic.bibtex.BibEntryWriter; import org.jabref.logic.bibtex.InvalidFieldValueException; @@ -51,7 +50,7 @@ import de.saxsys.mvvmfx.utils.validation.ObservableRuleBasedValidator; import de.saxsys.mvvmfx.utils.validation.ValidationMessage; -import org.controlsfx.control.NotificationPane; +import de.saxsys.mvvmfx.utils.validation.ValidationStatus; import org.fxmisc.flowless.VirtualizedScrollPane; import org.fxmisc.richtext.CodeArea; import org.slf4j.Logger; @@ -64,15 +63,17 @@ public class SourceTab extends EntryEditorTab { private final BibDatabaseMode mode; private final UndoManager undoManager; private final ObjectProperty sourceIsValid = new SimpleObjectProperty<>(); - @SuppressWarnings("unchecked") private final ObservableRuleBasedValidator sourceValidator = new ObservableRuleBasedValidator(sourceIsValid); + private final ObservableRuleBasedValidator sourceValidator = new ObservableRuleBasedValidator(); private final ImportFormatPreferences importFormatPreferences; private final FileUpdateMonitor fileMonitor; private final DialogService dialogService; private final StateManager stateManager; private Optional searchHighlightPattern = Optional.empty(); - private CodeArea codeArea; + private final CodeArea codeArea; private KeyBindingRepository keyBindingRepository; + private BibEntry previousEntry; + private class EditAction extends SimpleCommand { private final StandardActions command; @@ -81,7 +82,6 @@ private class EditAction extends SimpleCommand { @Override public void execute() { - if (codeArea != null) { switch (command) { case COPY: codeArea.copy(); @@ -97,7 +97,6 @@ public void execute() { break; } codeArea.requestFocus(); - } } } @@ -113,6 +112,11 @@ public SourceTab(BibDatabaseContext bibDatabaseContext, CountingUndoManager undo this.dialogService = dialogService; this.stateManager = stateManager; this.keyBindingRepository = keyBindingRepository; + this.codeArea = new CodeArea(); + + setupSourceEditor(); + VirtualizedScrollPane scrollableCodeArea = new VirtualizedScrollPane<>(codeArea); + this.setContent(scrollableCodeArea); stateManager.activeSearchQueryProperty().addListener((observable, oldValue, newValue) -> { searchHighlightPattern = newValue.flatMap(SearchQuery::getPatternForWords); @@ -144,7 +148,7 @@ private static String getSourceString(BibEntry entry, BibDatabaseMode type, Late /* Work around for different input methods. * https://github.com/FXMisc/RichTextFX/issues/146 */ - private class InputMethodRequestsObject implements InputMethodRequests { + private static class InputMethodRequestsObject implements InputMethodRequests { @Override public String getSelectedText() { @@ -167,8 +171,7 @@ public Point2D getTextLocation(int offset) { } } - private CodeArea createSourceEditor() { - CodeArea codeArea = new CodeArea(); + private void setupSourceEditor() { codeArea.setWrapText(true); codeArea.setInputMethodRequests(new InputMethodRequestsObject()); codeArea.setOnInputMethodTextChanged(event -> { @@ -191,7 +194,21 @@ private CodeArea createSourceEditor() { contextMenu.getStyleClass().add("context-menu"); codeArea.setContextMenu(contextMenu); - return codeArea; + sourceValidator.addRule(sourceIsValid); + + sourceValidator.getValidationStatus().getMessages().addListener((InvalidationListener) c -> { + ValidationStatus sourceValidationStatus = sourceValidator.getValidationStatus(); + if (!sourceValidationStatus.isValid()) { + sourceValidationStatus.getHighestMessage().ifPresent(message -> + dialogService.showErrorDialogAndWait(message.getMessage())); + } + }); + + codeArea.focusedProperty().addListener((obs, oldValue, onFocus) -> { + if (!onFocus && currentEntry != null) { + storeSource(currentEntry, codeArea.textProperty().getValue()); + } + }); } @Override @@ -199,46 +216,31 @@ public boolean shouldShow(BibEntry entry) { return true; } - @Override - protected void bindToEntry(BibEntry entry) { - CodeArea codeArea = createSourceEditor(); - VirtualizedScrollPane node = new VirtualizedScrollPane<>(codeArea); - NotificationPane notificationPane = new NotificationPane(node); - notificationPane.setShowFromTop(false); - sourceValidator.getValidationStatus().getMessages().addListener((ListChangeListener) c -> { - if (sourceValidator.getValidationStatus().isValid()) { - notificationPane.hide(); - } else { - sourceValidator.getValidationStatus().getHighestMessage().ifPresent(validationMessage -> { - notificationPane.show(validationMessage.getMessage());//this seems not working - dialogService.showErrorDialogAndWait(validationMessage.getMessage()); - }); + private void updateCodeArea() { + DefaultTaskExecutor.runAndWaitInJavaFXThread(() -> { + codeArea.clear(); + try { + codeArea.appendText(getSourceString(currentEntry, mode, fieldFormatterPreferences)); + highlightSearchPattern(); + } catch (IOException ex) { + codeArea.setEditable(false); + codeArea.appendText(ex.getMessage() + "\n\n" + + Localization.lang("Correct the entry, and reopen editor to display/edit source.")); + LOGGER.debug("Incorrect entry", ex); } }); - this.setContent(codeArea); - this.codeArea = codeArea; - - // Store source for on focus out event in the source code (within its text area) - // and update source code for every change of entry field values - BindingsHelper.bindContentBidirectional(entry.getFieldsObservable(), codeArea.focusedProperty(), onFocus -> { - if (!onFocus) { - storeSource(entry, codeArea.textProperty().getValue()); - } - }, fields -> { - DefaultTaskExecutor.runAndWaitInJavaFXThread(() -> { - codeArea.clear(); - try { - codeArea.appendText(getSourceString(entry, mode, fieldFormatterPreferences)); - highlightSearchPattern(); - } catch (IOException ex) { - codeArea.setEditable(false); - codeArea.appendText(ex.getMessage() + "\n\n" + - Localization.lang("Correct the entry, and reopen editor to display/edit source.")); - LOGGER.debug("Incorrect entry", ex); - } - }); - }); + } + + @Override + protected void bindToEntry(BibEntry entry) { + if (previousEntry != null) { + storeSource(previousEntry, codeArea.textProperty().getValue()); + } + this.previousEntry = entry; + + updateCodeArea(); + entry.getFieldsObservable().addListener((InvalidationListener) listener -> updateCodeArea()); } private void storeSource(BibEntry outOfFocusEntry, String text) { diff --git a/src/main/java/org/jabref/gui/preferences/AdvancedTabViewModel.java b/src/main/java/org/jabref/gui/preferences/AdvancedTabViewModel.java index 663df383ee6..e573cc379de 100644 --- a/src/main/java/org/jabref/gui/preferences/AdvancedTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/AdvancedTabViewModel.java @@ -25,6 +25,7 @@ import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator; import de.saxsys.mvvmfx.utils.validation.ValidationMessage; import de.saxsys.mvvmfx.utils.validation.ValidationStatus; +import de.saxsys.mvvmfx.utils.validation.Validator; public class AdvancedTabViewModel implements PreferenceTabViewModel { private final BooleanProperty remoteServerProperty = new SimpleBooleanProperty(); @@ -39,11 +40,11 @@ public class AdvancedTabViewModel implements PreferenceTabViewModel { private final StringProperty proxyUsernameProperty = new SimpleStringProperty(""); private final StringProperty proxyPasswordProperty = new SimpleStringProperty(""); - private FunctionBasedValidator remotePortValidator; - private FunctionBasedValidator proxyHostnameValidator; - private FunctionBasedValidator proxyPortValidator; - private FunctionBasedValidator proxyUsernameValidator; - private FunctionBasedValidator proxyPasswordValidator; + private Validator remotePortValidator; + private Validator proxyHostnameValidator; + private Validator proxyPortValidator; + private Validator proxyUsernameValidator; + private Validator proxyPasswordValidator; private final DialogService dialogService; private final JabRefPreferences preferences; diff --git a/src/main/java/org/jabref/gui/preferences/AppearanceTabViewModel.java b/src/main/java/org/jabref/gui/preferences/AppearanceTabViewModel.java index b841c1956c2..f2ee9d7a59b 100644 --- a/src/main/java/org/jabref/gui/preferences/AppearanceTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/AppearanceTabViewModel.java @@ -16,6 +16,7 @@ import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator; import de.saxsys.mvvmfx.utils.validation.ValidationMessage; import de.saxsys.mvvmfx.utils.validation.ValidationStatus; +import de.saxsys.mvvmfx.utils.validation.Validator; public class AppearanceTabViewModel implements PreferenceTabViewModel { @@ -27,7 +28,7 @@ public class AppearanceTabViewModel implements PreferenceTabViewModel { private final DialogService dialogService; private final JabRefPreferences preferences; - private FunctionBasedValidator fontSizeValidator; + private Validator fontSizeValidator; private List restartWarnings = new ArrayList<>(); @@ -93,12 +94,10 @@ public void storeSettings() { @Override public boolean validateSettings() { - if (fontOverrideProperty.getValue()) { - if (!fontSizeValidator.getValidationStatus().isValid()) { - fontSizeValidator.getValidationStatus().getHighestMessage().ifPresent(message -> - dialogService.showErrorDialogAndWait(message.getMessage())); - return false; - } + if (fontOverrideProperty.getValue() && !fontSizeValidator.getValidationStatus().isValid()) { + fontSizeValidator.getValidationStatus().getHighestMessage().ifPresent(message -> + dialogService.showErrorDialogAndWait(message.getMessage())); + return false; } return true; } diff --git a/src/main/java/org/jabref/gui/preferences/FileTabViewModel.java b/src/main/java/org/jabref/gui/preferences/FileTabViewModel.java index 0bc123b300e..1333f6d3d9b 100644 --- a/src/main/java/org/jabref/gui/preferences/FileTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/FileTabViewModel.java @@ -27,6 +27,7 @@ import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator; import de.saxsys.mvvmfx.utils.validation.ValidationMessage; import de.saxsys.mvvmfx.utils.validation.ValidationStatus; +import de.saxsys.mvvmfx.utils.validation.Validator; public class FileTabViewModel implements PreferenceTabViewModel { @@ -51,7 +52,7 @@ public class FileTabViewModel implements PreferenceTabViewModel { private final BooleanProperty autosaveLocalLibraries = new SimpleBooleanProperty(); - private final FunctionBasedValidator mainFileDirValidator; + private final Validator mainFileDirValidator; private final DialogService dialogService; private final JabRefPreferences preferences; @@ -136,9 +137,10 @@ ValidationStatus mainFileDirValidationStatus() { @Override public boolean validateSettings() { - ValidationStatus status = mainFileDirValidationStatus(); - if (!status.isValid()) { - dialogService.showErrorDialogAndWait(status.getHighestMessage().get().getMessage()); + ValidationStatus validationStatus = mainFileDirValidationStatus(); + if (!validationStatus.isValid()) { + validationStatus.getHighestMessage().ifPresent(message -> + dialogService.showErrorDialogAndWait(message.getMessage())); return false; } return true; diff --git a/src/main/java/org/jabref/gui/preferences/GeneralTabViewModel.java b/src/main/java/org/jabref/gui/preferences/GeneralTabViewModel.java index 05c2a7afcb9..ff32611f19a 100644 --- a/src/main/java/org/jabref/gui/preferences/GeneralTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/GeneralTabViewModel.java @@ -25,6 +25,7 @@ import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator; import de.saxsys.mvvmfx.utils.validation.ValidationMessage; import de.saxsys.mvvmfx.utils.validation.ValidationStatus; +import de.saxsys.mvvmfx.utils.validation.Validator; public class GeneralTabViewModel implements PreferenceTabViewModel { private final ListProperty languagesListProperty = new SimpleListProperty<>(); @@ -50,7 +51,7 @@ public class GeneralTabViewModel implements PreferenceTabViewModel { private final StringProperty markTimeStampFieldNameProperty = new SimpleStringProperty(""); private final BooleanProperty updateTimeStampProperty = new SimpleBooleanProperty(); - private FunctionBasedValidator markTimeStampFormatValidator; + private Validator markTimeStampFormatValidator; private final DialogService dialogService; private final JabRefPreferences preferences; diff --git a/src/main/java/org/jabref/gui/preferences/PreviewTabViewModel.java b/src/main/java/org/jabref/gui/preferences/PreviewTabViewModel.java index e25f26e4260..0b767b392dc 100644 --- a/src/main/java/org/jabref/gui/preferences/PreviewTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/PreviewTabViewModel.java @@ -43,6 +43,7 @@ import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator; import de.saxsys.mvvmfx.utils.validation.ValidationMessage; import de.saxsys.mvvmfx.utils.validation.ValidationStatus; +import de.saxsys.mvvmfx.utils.validation.Validator; import org.fxmisc.richtext.model.StyleSpans; import org.fxmisc.richtext.model.StyleSpansBuilder; import org.slf4j.Logger; @@ -65,7 +66,7 @@ public class PreviewTabViewModel implements PreferenceTabViewModel { private final PreviewPreferences previewPreferences; private final TaskExecutor taskExecutor; private final CustomLocalDragboard localDragboard = GUIGlobals.localDragboard; - private FunctionBasedValidator chosenListValidator; + private Validator chosenListValidator; private ListProperty dragSourceList = null; public PreviewTabViewModel(DialogService dialogService, JabRefPreferences preferences, TaskExecutor taskExecutor) { diff --git a/src/main/java/org/jabref/gui/preferences/TableColumnsTabViewModel.java b/src/main/java/org/jabref/gui/preferences/TableColumnsTabViewModel.java index 52331aba2db..545bd3c3d15 100644 --- a/src/main/java/org/jabref/gui/preferences/TableColumnsTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/TableColumnsTabViewModel.java @@ -32,6 +32,7 @@ import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator; import de.saxsys.mvvmfx.utils.validation.ValidationMessage; import de.saxsys.mvvmfx.utils.validation.ValidationStatus; +import de.saxsys.mvvmfx.utils.validation.Validator; public class TableColumnsTabViewModel implements PreferenceTabViewModel { @@ -60,7 +61,7 @@ public MainTableColumnModel fromString(String string) { private final BooleanProperty specialFieldsSerializeProperty = new SimpleBooleanProperty(); private final BooleanProperty extraFileColumnsEnabledProperty = new SimpleBooleanProperty(); - private FunctionBasedValidator columnsNotEmptyValidator; + private Validator columnsNotEmptyValidator; private List restartWarnings = new ArrayList<>(); @@ -239,9 +240,10 @@ ValidationStatus columnsListValidationStatus() { @Override public boolean validateSettings() { - ValidationStatus status = columnsListValidationStatus(); - if (!status.isValid() && status.getHighestMessage().isPresent()) { - dialogService.showErrorDialogAndWait(status.getHighestMessage().get().getMessage()); + ValidationStatus validationStatus = columnsListValidationStatus(); + if (!validationStatus.isValid()) { + validationStatus.getHighestMessage().ifPresent(message -> + dialogService.showErrorDialogAndWait(message.getMessage())); return false; } return true; diff --git a/src/main/java/org/jabref/gui/preferences/XmpPrivacyTabViewModel.java b/src/main/java/org/jabref/gui/preferences/XmpPrivacyTabViewModel.java index 8627d81460c..6ddd04cecae 100644 --- a/src/main/java/org/jabref/gui/preferences/XmpPrivacyTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/XmpPrivacyTabViewModel.java @@ -21,6 +21,7 @@ import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator; import de.saxsys.mvvmfx.utils.validation.ValidationMessage; import de.saxsys.mvvmfx.utils.validation.ValidationStatus; +import de.saxsys.mvvmfx.utils.validation.Validator; public class XmpPrivacyTabViewModel implements PreferenceTabViewModel { @@ -32,7 +33,7 @@ public class XmpPrivacyTabViewModel implements PreferenceTabViewModel { private final DialogService dialogService; private final JabRefPreferences preferences; - private FunctionBasedValidator xmpFilterListValidator; + private Validator xmpFilterListValidator; XmpPrivacyTabViewModel(DialogService dialogService, JabRefPreferences preferences) { this.dialogService = dialogService; diff --git a/src/main/java/org/jabref/gui/texparser/ParseTexDialogViewModel.java b/src/main/java/org/jabref/gui/texparser/ParseTexDialogViewModel.java index e64d921ba8a..19ae077e7f3 100644 --- a/src/main/java/org/jabref/gui/texparser/ParseTexDialogViewModel.java +++ b/src/main/java/org/jabref/gui/texparser/ParseTexDialogViewModel.java @@ -37,6 +37,7 @@ import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator; import de.saxsys.mvvmfx.utils.validation.ValidationMessage; import de.saxsys.mvvmfx.utils.validation.ValidationStatus; +import de.saxsys.mvvmfx.utils.validation.Validator; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -50,7 +51,7 @@ public class ParseTexDialogViewModel extends AbstractViewModel { private final PreferencesService preferencesService; private final FileUpdateMonitor fileMonitor; private final StringProperty texDirectory; - private final FunctionBasedValidator texDirectoryValidator; + private final Validator texDirectoryValidator; private final ObjectProperty root; private final ObservableList> checkedFileList; private final BooleanProperty noFilesFound; diff --git a/src/test/java/org/jabref/gui/entryeditor/SourceTabTest.java b/src/test/java/org/jabref/gui/entryeditor/SourceTabTest.java index a65bc74ed5d..320b2f37c34 100644 --- a/src/test/java/org/jabref/gui/entryeditor/SourceTabTest.java +++ b/src/test/java/org/jabref/gui/entryeditor/SourceTabTest.java @@ -78,7 +78,7 @@ void switchingFromSourceTabDoesNotThrowException(FxRobot robot) throws Exception // Update source editor robot.interact(() -> pane.getSelectionModel().select(2)); - robot.interact(() -> sourceTab.bindToEntry(entry)); + robot.interact(() -> sourceTab.notifyAboutFocus(entry)); robot.clickOn(1200, 500); robot.interrupt(100);