Various improvements on JournalsTab in the preferences (#7937)

* Moved remove button in prefs journals tab to the table for ui consistency * Removed PseudoAbbreviation in new file and fixed typo * Reworked journal abbreviations tab in prefs layout * Refactored JournalAbbreviationsTab * Slight improvements * CHANGELOG.md and undone removal of l10n string * Hid disabled remove buttons and reworded * Forgotten rewording * Adapted IntelliJ suggestions in tests, simplified * Fixed test, added logger message on store failure * Parameterized JournalAbbreviationsViewModelTabTest * use list in test Co-authored-by: Siedlerchr <siedlerkiller@gmail.com>
JabRef · Jul 30, 2021 · 399e117 · 399e117
1 parent 2bec2a8
commit 399e117
Show file tree

Hide file tree

Showing 12 changed files with 316 additions and 1,225 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 
 ### Changed
 
+- We slightly changed the layout of the Journal tab in the preferences for ui consistency. [#7937](https://github.com/JabRef/jabref/pull/7937)
 - The JabRefHost on Windows now writes a temporary file and calls `-importToOpen` instead of passing the bibtex via `-importBibtex`. [#7374](https://github.com/JabRef/jabref/issues/7374), [JabRef Browser Ext #274](https://github.com/JabRef/JabRef-Browser-Extension/issues/274)
 
 ### Fixed

diff --git a/src/main/java/org/jabref/gui/icon/IconTheme.java b/src/main/java/org/jabref/gui/icon/IconTheme.java
@@ -322,9 +322,9 @@ public enum JabRefIcons implements JabRefIcon {
         LATEX_COMMENT(MaterialDesignC.COMMENT_TEXT_OUTLINE),
         LATEX_LINE(MaterialDesignF.FORMAT_LINE_SPACING),
         PASSWORD_REVEALED(MaterialDesignE.EYE),
-        ADD_ABBREVIATION_LIST(MaterialDesignF.FOLDER_PLUS),
+        ADD_ABBREVIATION_LIST(MaterialDesignP.PLUS),
         OPEN_ABBREVIATION_LIST(MaterialDesignF.FOLDER_OUTLINE),
-        REMOVE_ABBREVIATION_LIST(MaterialDesignF.FOLDER_REMOVE),
+        REMOVE_ABBREVIATION_LIST(MaterialDesignM.MINUS),
         ADD_ABBREVIATION(MaterialDesignP.PLAYLIST_PLUS),
         REMOVE_ABBREVIATION(MaterialDesignP.PLAYLIST_MINUS),
         NEW_ENTRY_FROM_PLAIN_TEXT(MaterialDesignP.PLUS_BOX),

diff --git a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java
@@ -35,7 +35,6 @@ public AbbreviationsFileViewModel(Path filePath) {
         this.path = Optional.ofNullable(filePath);
         this.name = path.get().toAbsolutePath().toString();
         this.isBuiltInList = new SimpleBooleanProperty(false);
-        this.abbreviations.add(new AbbreviationViewModel(null));
     }
 
     /**

diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.css b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.css
diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.fxml b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.fxml
@@ -1,5 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 
+<?import javafx.geometry.Insets?>
 <?import javafx.scene.control.Button?>
 <?import javafx.scene.control.ComboBox?>
 <?import javafx.scene.control.Label?>
@@ -8,67 +9,75 @@
 <?import javafx.scene.control.TableView?>
 <?import javafx.scene.control.Tooltip?>
 <?import javafx.scene.layout.HBox?>
-<?import javafx.scene.layout.Pane?>
 <?import javafx.scene.layout.VBox?>
+<?import org.jabref.gui.icon.JabRefIconView?>
 <?import org.controlsfx.control.textfield.CustomTextField?>
-<?import javafx.geometry.Insets?>
 <fx:root spacing="10.0" type="VBox"
          xmlns="http://javafx.com/javafx" xmlns:fx="http://javafx.com/fxml"
          fx:controller="org.jabref.gui.preferences.journals.JournalAbbreviationsTab">
 
     <Label styleClass="titleHeader" text="%Journal abbreviations"/>
 
-    <HBox styleClass="custom-header">
-        <Button fx:id="addAbbreviationListButton" onAction="#addList" styleClass="icon">
+    <HBox spacing="4.0" alignment="CENTER_LEFT">
+        <Label text="%Journal lists:"/>
+        <ComboBox fx:id="journalFilesBox" promptText="%No abbreviation files loaded"
+                  minWidth="200.0" maxWidth="Infinity" HBox.hgrow="ALWAYS"/>
+        <Button onAction="#addList" styleClass="icon-button,narrow">
+            <graphic>
+                <JabRefIconView glyph="ADD_ABBREVIATION_LIST"/>
+            </graphic>
             <tooltip>
                 <Tooltip autoHide="true" text="%Add new list"/>
             </tooltip>
         </Button>
-        <Button fx:id="openAbbreviationListButton" onAction="#openList" styleClass="icon">
+        <Button onAction="#openList" styleClass="icon-button,narrow">
+            <graphic>
+                <JabRefIconView glyph="OPEN_ABBREVIATION_LIST"/>
+            </graphic>
             <tooltip>
                 <Tooltip autoHide="true" text="%Open existing list"/>
             </tooltip>
         </Button>
-        <Button fx:id="removeAbbreviationListButton" onAction="#removeList"
-                styleClass="icon">
+        <Button fx:id="removeAbbreviationListButton" onAction="#removeList" styleClass="icon-button,narrow">
+            <graphic>
+                <JabRefIconView glyph="REMOVE_ABBREVIATION_LIST"/>
+            </graphic>
             <tooltip>
                 <Tooltip autoHide="true" text="%Remove list"/>
             </tooltip>
         </Button>
-        <Pane HBox.hgrow="ALWAYS"/>
-        <ComboBox fx:id="journalFilesBox" minWidth="200.0" maxWidth="Infinity" promptText="%No abbreviation files loaded"
-                  HBox.hgrow="ALWAYS"/>
-        <Pane HBox.hgrow="ALWAYS"/>
-        <Button fx:id="addAbbreviationButton" onAction="#addAbbreviation" styleClass="icon">
-            <tooltip>
-                <Tooltip autoHide="true" text="%Add abbreviation"/>
-            </tooltip>
-        </Button>
-        <Button fx:id="removeAbbreviationButton" onAction="#removeAbbreviation" styleClass="icon">
-            <tooltip>
-                <Tooltip autoHide="true" text="%Remove abbreviation"/>
-            </tooltip>
-        </Button>
     </HBox>
 
-    <TableView fx:id="journalAbbreviationsTable" editable="true" styleClass="abbreviations-table">
-        <columns>
-            <TableColumn fx:id="journalTableNameColumn" prefWidth="400.0" text="%Full journal name"/>
-            <TableColumn fx:id="journalTableAbbreviationColumn" prefWidth="200.0" text="%Abbreviation name"/>
-            <TableColumn fx:id="journalTableShortestUniqueAbbreviationColumn" prefWidth="200.0"
-                         text="%Shortest unique abbreviation"/>
-        </columns>
-        <columnResizePolicy>
-            <TableView fx:constant="CONSTRAINED_RESIZE_POLICY"/>
-        </columnResizePolicy>
-        <placeholder>
-            <Label fx:id="loadingLabel" maxHeight="30.0" text="%Loading built in lists"/>
-            <ProgressIndicator fx:id="progressIndicator" maxHeight="30.0" opacity="0.4"/>
-        </placeholder>
-    </TableView>
-    <CustomTextField fx:id="searchBox" promptText="%Search" VBox.vgrow="NEVER">
-        <VBox.margin>
-            <Insets bottom="3.0" left="3.0" right="3.0" top="3.0"/>
-        </VBox.margin>
-    </CustomTextField>
+    <VBox spacing="10.0" HBox.hgrow="ALWAYS">
+        <CustomTextField fx:id="searchBox" promptText="%Filter" VBox.vgrow="NEVER">
+            <VBox.margin>
+                <Insets bottom="3.0" left="3.0" right="3.0" top="3.0"/>
+            </VBox.margin>
+        </CustomTextField>
+        <TableView fx:id="journalAbbreviationsTable" editable="true" styleClass="abbreviations-table">
+            <columns>
+                <TableColumn fx:id="journalTableNameColumn" prefWidth="400.0" text="%Full journal name"/>
+                <TableColumn fx:id="journalTableAbbreviationColumn" prefWidth="200.0" text="%Abbreviation name"/>
+                <TableColumn fx:id="journalTableShortestUniqueAbbreviationColumn" prefWidth="200.0"
+                             text="%Shortest unique abbreviation"/>
+                <TableColumn fx:id="actionsColumn" minWidth="30.0" prefWidth="30.0" maxWidth="30.0"
+                             resizable="false" reorderable="false"/>
+            </columns>
+            <columnResizePolicy>
+                <TableView fx:constant="CONSTRAINED_RESIZE_POLICY"/>
+            </columnResizePolicy>
+            <placeholder>
+                <Label fx:id="loadingLabel" maxHeight="30.0" text="%Loading built in lists"/>
+                <ProgressIndicator fx:id="progressIndicator" maxHeight="30.0" opacity="0.4"/>
+            </placeholder>
+        </TableView>
+    </VBox>
+
+    <HBox alignment="BASELINE_RIGHT" spacing="10.0">
+        <Button fx:id="addAbbreviationButton" onAction="#addAbbreviation" text="%Add abbreviation">
+            <graphic>
+                <JabRefIconView glyph="ADD_NOBOX"/>
+            </graphic>
+        </Button>
+    </HBox>
 </fx:root>
diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java
@@ -28,6 +28,7 @@
 import org.jabref.gui.preferences.PreferencesTab;
 import org.jabref.gui.util.ColorUtil;
 import org.jabref.gui.util.TaskExecutor;
+import org.jabref.gui.util.ValueTableCellFactory;
 import org.jabref.logic.journals.JournalAbbreviationRepository;
 import org.jabref.logic.l10n.Localization;
 
@@ -43,16 +44,17 @@ public class JournalAbbreviationsTab extends AbstractPreferenceTabView<JournalAb
 
     @FXML private Label loadingLabel;
     @FXML private ProgressIndicator progressIndicator;
+
     @FXML private TableView<AbbreviationViewModel> journalAbbreviationsTable;
     @FXML private TableColumn<AbbreviationViewModel, String> journalTableNameColumn;
     @FXML private TableColumn<AbbreviationViewModel, String> journalTableAbbreviationColumn;
     @FXML private TableColumn<AbbreviationViewModel, String> journalTableShortestUniqueAbbreviationColumn;
+    @FXML private TableColumn<AbbreviationViewModel, String> actionsColumn;
+
     private FilteredList<AbbreviationViewModel> filteredAbbreviations;
     @FXML private ComboBox<AbbreviationsFileViewModel> journalFilesBox;
+
     @FXML private Button addAbbreviationButton;
-    @FXML private Button removeAbbreviationButton;
-    @FXML private Button openAbbreviationListButton;
-    @FXML private Button addAbbreviationListButton;
     @FXML private Button removeAbbreviationListButton;
 
     @FXML private CustomTextField searchBox;
@@ -61,8 +63,6 @@ public class JournalAbbreviationsTab extends AbstractPreferenceTabView<JournalAb
     @Inject private JournalAbbreviationRepository abbreviationRepository;
 
     private Timeline invalidateSearch;
-    private ObjectProperty<Color> flashingColor;
-    private StringProperty flashingColorStringProperty;
 
     public JournalAbbreviationsTab() {
         ViewLoader.view(this)
@@ -76,7 +76,6 @@ private void initialize() {
 
         filteredAbbreviations = new FilteredList<>(viewModel.abbreviationsProperty());
 
-        setButtonStyles();
         setUpTable();
         setBindings();
         setAnimations();
@@ -85,14 +84,6 @@ private void initialize() {
         searchBox.setLeft(IconTheme.JabRefIcons.SEARCH.getGraphicNode());
     }
 
-    private void setButtonStyles() {
-        addAbbreviationListButton.setGraphic(IconTheme.JabRefIcons.ADD_ABBREVIATION_LIST.getGraphicNode());
-        openAbbreviationListButton.setGraphic(IconTheme.JabRefIcons.OPEN_ABBREVIATION_LIST.getGraphicNode());
-        removeAbbreviationListButton.setGraphic(IconTheme.JabRefIcons.REMOVE_ABBREVIATION_LIST.getGraphicNode());
-        addAbbreviationButton.setGraphic(IconTheme.JabRefIcons.ADD_ABBREVIATION.getGraphicNode());
-        removeAbbreviationButton.setGraphic(IconTheme.JabRefIcons.REMOVE_ABBREVIATION.getGraphicNode());
-    }
-
     private void setUpTable() {
         journalTableNameColumn.setCellValueFactory(cellData -> cellData.getValue().nameProperty());
         journalTableNameColumn.setCellFactory(TextFieldTableCell.forTableColumn());
@@ -102,6 +93,16 @@ private void setUpTable() {
 
         journalTableShortestUniqueAbbreviationColumn.setCellValueFactory(cellData -> cellData.getValue().shortestUniqueAbbreviationProperty());
         journalTableShortestUniqueAbbreviationColumn.setCellFactory(TextFieldTableCell.forTableColumn());
+
+        actionsColumn.setCellValueFactory(cellData -> cellData.getValue().nameProperty());
+        new ValueTableCellFactory<AbbreviationViewModel, String>()
+                .withGraphic(name -> IconTheme.JabRefIcons.DELETE_ENTRY.getGraphicNode())
+                .withTooltip(name -> Localization.lang("Remove journal '%0'", name))
+                .withDisableExpression(item -> viewModel.isEditableAndRemovableProperty().not())
+                .withVisibleExpression(item -> viewModel.isEditableAndRemovableProperty())
+                .withOnMouseClickedEvent(item -> evt ->
+                        viewModel.removeAbbreviation(journalAbbreviationsTable.getFocusModel().getFocusedItem()))
+                .install(actionsColumn);
     }
 
     private void setBindings() {
@@ -121,19 +122,19 @@ private void setBindings() {
         journalFilesBox.valueProperty().bindBidirectional(viewModel.currentFileProperty());
 
         addAbbreviationButton.disableProperty().bind(viewModel.isEditableAndRemovableProperty().not());
-        removeAbbreviationButton.disableProperty().bind(viewModel.isAbbreviationEditableAndRemovable().not());
 
         loadingLabel.visibleProperty().bind(viewModel.isLoadingProperty());
         progressIndicator.visibleProperty().bind(viewModel.isLoadingProperty());
 
         searchBox.textProperty().addListener((observable, previousText, searchTerm) -> {
-            filteredAbbreviations.setPredicate(abbreviation -> searchTerm.isEmpty() ? true : abbreviation.containsCaseIndependent(searchTerm));
+            filteredAbbreviations.setPredicate(abbreviation -> searchTerm.isEmpty() || abbreviation.containsCaseIndependent(searchTerm));
         });
     }
 
     private void setAnimations() {
-        flashingColor = new SimpleObjectProperty<>(Color.TRANSPARENT);
-        flashingColorStringProperty = createFlashingColorStringProperty(flashingColor);
+        ObjectProperty<Color> flashingColor = new SimpleObjectProperty<>(Color.TRANSPARENT);
+        StringProperty flashingColorStringProperty = createFlashingColorStringProperty(flashingColor);
+
         searchBox.styleProperty().bind(
                 new SimpleStringProperty("-fx-control-inner-background: ").concat(flashingColorStringProperty).concat(";")
         );
@@ -196,11 +197,6 @@ private void editAbbreviation() {
                 journalTableNameColumn);
     }
 
-    @FXML
-    private void removeAbbreviation() {
-        viewModel.deleteAbbreviation();
-    }
-
     private void selectNewAbbreviation() {
         int lastRow = viewModel.abbreviationsCountProperty().get() - 1;
         journalAbbreviationsTable.scrollTo(lastRow);