UI consistency - BibTexStringEditorDialog rework

src/main/java/org/jabref/gui/Base.css

@@ -613,6 +613,11 @@
     -fx-text-fill: -jr-black;
 }
 
+.table-cell:invalid,
+.list-cell:invalid {
+        -fx-background-color: -jr-warn;
+}
+
 .combo-box-base {
     -fx-background-color: -fx-outer-border, -fx-control-inner-background;
     -fx-background-insets: 0, 1;

src/main/java/org/jabref/gui/entryeditor/EntryEditor.css

@@ -86,10 +86,6 @@
     -fx-background-color: -jr-error;
 }
 
-.list-cell:invalid {
-    -fx-background-color: -jr-warn;
-}
-
 .code-area .context-menu {
     -fx-font-family: sans-serif;
 }

src/main/java/org/jabref/gui/metadata/BibtexStringEditorDialog.fxml

@@ -7,31 +7,49 @@
 <?import javafx.scene.control.TableView?>
 <?import javafx.scene.layout.HBox?>
 <?import javafx.scene.layout.VBox?>
-<DialogPane minHeight="-Infinity" minWidth="-Infinity" prefHeight="340.0" prefWidth="427.0" xmlns="http://javafx.com/javafx/8.0.171" xmlns:fx="http://javafx.com/fxml/1" fx:controller="org.jabref.gui.metadata.BibtexStringEditorDialogView">
-   <content>
-      <VBox>
-         <children>
-            <TableView fx:id="tblStrings" prefHeight="250.0">
-              <columns>
-                <TableColumn fx:id="colLabel" prefWidth="75.0" text="%Label" />
-                <TableColumn fx:id="colContent" prefWidth="75.0" text="%Content" />
-              </columns>
-               <columnResizePolicy>
-                  <TableView fx:constant="CONSTRAINED_RESIZE_POLICY" />
-               </columnResizePolicy>
-            </TableView>
-            <HBox minWidth="-Infinity" prefHeight="100.0" prefWidth="300.0">
-               <children>
-                <Button fx:id="btnNewString" minWidth="-Infinity" mnemonicParsing="false" onAction="#addString" text="%Add new String" HBox.hgrow="ALWAYS" />
-                  <Button fx:id="btnRemove" mnemonicParsing="false" onAction="#removeString" text="%Remove selected Strings" HBox.hgrow="ALWAYS" />
-                  <Button fx:id="btnHelp" mnemonicParsing="false" onAction="#openHelp" HBox.hgrow="ALWAYS" />
-               </children>
-            </HBox>
-         </children>
-      </VBox>
-   </content>
-   <buttonTypes>
-     <ButtonType fx:constant="CANCEL" />
-     <ButtonType fx:id="saveButton" buttonData="OK_DONE" text="%Save" />
-   </buttonTypes>
+<?import javafx.scene.control.Tooltip?>
+<?import org.jabref.gui.icon.JabRefIconView?>
+
+<DialogPane minHeight="-Infinity" minWidth="-Infinity" prefHeight="350.0" prefWidth="450.0"
+            xmlns="http://javafx.com/javafx/8.0.171" xmlns:fx="http://javafx.com/fxml/1"
+            fx:controller="org.jabref.gui.metadata.BibtexStringEditorDialogView">
+    <content>
+        <HBox spacing="4.0">
+            <VBox spacing="4.0" HBox.hgrow="ALWAYS">
+                <TableView fx:id="stringsList">
+                    <columns>
+                        <TableColumn fx:id="labelColumn" minWidth="75.0" text="%Label"/>
+                        <TableColumn fx:id="contentColumn" minWidth="75.0" text="%Content"/>
+                        <TableColumn fx:id="actionsColumn" maxWidth="30.0" minWidth="30.0" prefWidth="30.0"
+                                     editable="false" resizable="false" reorderable="false"/>
+                    </columns>
+                    <columnResizePolicy>
+                        <TableView fx:constant="CONSTRAINED_RESIZE_POLICY"/>
+                    </columnResizePolicy>
+                </TableView>
+            </VBox>
+            <VBox>
+                <Button styleClass="icon-button,narrow" onAction="#openHelp">
+                    <graphic>
+                        <JabRefIconView glyph="HELP"/>
+                    </graphic>
+                    <tooltip>
+                        <Tooltip text="%Open Help page"/>
+                    </tooltip>
+                </Button>
+                <VBox VBox.vgrow="ALWAYS"/>
+                <Button styleClass="icon-button,narrow" fx:id="addStringButton" minWidth="-Infinity"
+                        onAction="#addString">
+                    <graphic>
+                        <JabRefIconView glyph="ADD_NOBOX"/>
+                    </graphic>
+                    <tooltip>
+                        <Tooltip text="%Add new String"/>
+                    </tooltip>
+                </Button>
+            </VBox>
+        </HBox>
+    </content>
+    <ButtonType fx:constant="CANCEL"/>
+    <ButtonType fx:id="saveButton" buttonData="OK_DONE" text="%Save"/>
 </DialogPane>

src/main/java/org/jabref/gui/metadata/BibtexStringEditorDialogView.java

@@ -1,8 +1,9 @@
 package org.jabref.gui.metadata;
 
+import java.util.Optional;
+
 import javax.inject.Inject;
 
-import javafx.event.ActionEvent;
 import javafx.fxml.FXML;
 import javafx.scene.control.Button;
 import javafx.scene.control.ButtonType;
@@ -13,34 +14,30 @@
 import javafx.util.converter.DefaultStringConverter;
 
 import org.jabref.gui.DialogService;
-import org.jabref.gui.icon.IconTheme.JabRefIcons;
+import org.jabref.gui.icon.IconTheme;
 import org.jabref.gui.util.BaseDialog;
-import org.jabref.gui.util.IconValidationDecorator;
+import org.jabref.gui.util.ValueTableCellFactory;
 import org.jabref.gui.util.ViewModelTextFieldTableCellVisualizationFactory;
 import org.jabref.logic.l10n.Localization;
 import org.jabref.model.database.BibDatabase;
 
 import com.airhacks.afterburner.views.ViewLoader;
-import de.saxsys.mvvmfx.utils.validation.visualization.ControlsFxVisualizer;
 
 public class BibtexStringEditorDialogView extends BaseDialog<Void> {
 
-    @FXML private Button btnNewString;
-    @FXML private Button btnRemove;
-    @FXML private Button btnHelp;
+    @FXML private TableView<BibtexStringEditorItemModel> stringsList;
+    @FXML private TableColumn<BibtexStringEditorItemModel, String> labelColumn;
+    @FXML private TableColumn<BibtexStringEditorItemModel, String> contentColumn;
+    @FXML private TableColumn<BibtexStringEditorItemModel, String> actionsColumn;
+    @FXML private Button addStringButton;
     @FXML private ButtonType saveButton;
 
-    @FXML private TableView<BibtexStringViewModel> tblStrings;
-    @FXML private TableColumn<BibtexStringViewModel, String> colLabel;
-    @FXML private TableColumn<BibtexStringViewModel, String> colContent;
-
-    private final ControlsFxVisualizer visualizer = new ControlsFxVisualizer();
     private final BibtexStringEditorDialogViewModel viewModel;
 
     @Inject private DialogService dialogService;
 
     public BibtexStringEditorDialogView(BibDatabase database) {
-        viewModel = new BibtexStringEditorDialogViewModel(database);
+        this.viewModel = new BibtexStringEditorDialogViewModel(database);
 
         ViewLoader.view(this)
                   .load()
@@ -62,53 +59,67 @@ public BibtexStringEditorDialogView(BibDatabase database) {
 
     @FXML
     private void initialize() {
-        visualizer.setDecoration(new IconValidationDecorator());
-
-        btnHelp.setGraphic(JabRefIcons.HELP.getGraphicNode());
-        btnHelp.setTooltip(new Tooltip(Localization.lang("Open Help page")));
+        addStringButton.setTooltip(new Tooltip(Localization.lang("New string")));
 
-        btnNewString.setTooltip(new Tooltip(Localization.lang("New string")));
-        btnRemove.setTooltip(new Tooltip(Localization.lang("Remove selected strings")));
+        labelColumn.setSortable(true);
+        labelColumn.setReorderable(false);
 
-        colLabel.setCellValueFactory(cellData -> cellData.getValue().labelProperty());
-        new ViewModelTextFieldTableCellVisualizationFactory<BibtexStringViewModel, String>().withValidation(BibtexStringViewModel::labelValidation, visualizer).install(colLabel, new DefaultStringConverter());
+        labelColumn.setCellValueFactory(cellData -> cellData.getValue().labelProperty());
+        new ViewModelTextFieldTableCellVisualizationFactory<BibtexStringEditorItemModel, String>()
+                .withValidation(BibtexStringEditorItemModel::labelValidation)
+                .install(labelColumn, new DefaultStringConverter());
+        labelColumn.setOnEditCommit((CellEditEvent<BibtexStringEditorItemModel, String> cellEvent) -> {
 
-        colContent.setCellValueFactory(cellData -> cellData.getValue().contentProperty());
-        new ViewModelTextFieldTableCellVisualizationFactory<BibtexStringViewModel, String>().withValidation(BibtexStringViewModel::contentValidation, visualizer).install(colContent, new DefaultStringConverter());
+            BibtexStringEditorItemModel cellItem = cellEvent.getTableView()
+                                                            .getItems()
+                                                            .get(cellEvent.getTablePosition().getRow());
 
-        colLabel.setOnEditCommit((CellEditEvent<BibtexStringViewModel, String> cell) -> {
+            Optional<BibtexStringEditorItemModel> existingItem = viewModel.labelAlreadyExists(cellEvent.getNewValue());
 
-            String newLabelValue = cell.getNewValue();
-            if (cell.getTableView().getItems().stream().anyMatch(strs -> strs.labelProperty().get().equals(newLabelValue))) {
+            if (existingItem.isPresent() && !existingItem.get().equals(cellItem)) {
+                dialogService.showErrorDialogAndWait(Localization.lang(
+                        "A string with the label '%0' already exists.",
+                        cellEvent.getNewValue()));
 
-                dialogService.showErrorDialogAndWait(Localization.lang("A string with the label '%0' already exists.", newLabelValue));
-                cell.getRowValue().setLabel("");
+                cellItem.setLabel(cellEvent.getOldValue());
             } else {
-                cell.getRowValue().setLabel(cell.getNewValue());
+                cellItem.setLabel(cellEvent.getNewValue());
             }
-        });
-        colContent.setOnEditCommit((CellEditEvent<BibtexStringViewModel, String> cell) -> {
-            cell.getRowValue().setContent(cell.getNewValue());
-        });
 
-        tblStrings.itemsProperty().bindBidirectional(viewModel.allStringsProperty());
-        tblStrings.setEditable(true);
+            cellEvent.getTableView().refresh();
+        });
 
-        viewModel.seletedItemProperty().bind(tblStrings.getSelectionModel().selectedItemProperty());
+        contentColumn.setSortable(true);
+        contentColumn.setReorderable(false);
+        contentColumn.setCellValueFactory(cellData -> cellData.getValue().contentProperty());
+        new ViewModelTextFieldTableCellVisualizationFactory<BibtexStringEditorItemModel, String>()
+                .withValidation(BibtexStringEditorItemModel::contentValidation)
+                .install(contentColumn, new DefaultStringConverter());
+        contentColumn.setOnEditCommit((CellEditEvent<BibtexStringEditorItemModel, String> cell) ->
+                cell.getRowValue().setContent(cell.getNewValue()));
+
+        actionsColumn.setSortable(false);
+        actionsColumn.setReorderable(false);
+        actionsColumn.setCellValueFactory(cellData -> cellData.getValue().labelProperty());
+        new ValueTableCellFactory<BibtexStringEditorItemModel, String>()
+                .withGraphic(label -> IconTheme.JabRefIcons.DELETE_ENTRY.getGraphicNode())
+                .withTooltip(label -> Localization.lang("Remove string %0", label))
+                .withOnMouseClickedEvent(item -> evt ->
+                        viewModel.removeString(stringsList.getFocusModel().getFocusedItem()))
+                .install(actionsColumn);
+
+        stringsList.itemsProperty().bindBidirectional(viewModel.stringsListProperty());
+        stringsList.setEditable(true);
     }
 
     @FXML
-    private void addString(ActionEvent event) {
+    private void addString() {
         viewModel.addNewString();
+        stringsList.edit(stringsList.getItems().size() - 1, labelColumn);
     }
 
     @FXML
-    private void openHelp(ActionEvent event) {
+    private void openHelp() {
         viewModel.openHelpPage();
     }
-
-    @FXML
-    private void removeString(ActionEvent event) {
-        viewModel.removeString();
-    }
 }

src/main/java/org/jabref/gui/metadata/BibtexStringEditorDialogViewModel.java

@@ -1,15 +1,12 @@
 package org.jabref.gui.metadata;
 
-import java.util.List;
-import java.util.Set;
+import java.util.Optional;
 import java.util.stream.Collectors;
 
 import javafx.beans.property.BooleanProperty;
 import javafx.beans.property.ListProperty;
-import javafx.beans.property.ObjectProperty;
 import javafx.beans.property.SimpleBooleanProperty;
 import javafx.beans.property.SimpleListProperty;
-import javafx.beans.property.SimpleObjectProperty;
 import javafx.beans.value.ObservableValue;
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
@@ -24,67 +21,80 @@
 import org.fxmisc.easybind.EasyBind;
 
 public class BibtexStringEditorDialogViewModel extends AbstractViewModel {
+    private static final String NEW_STRING_LABEL = "NewString"; // must not contain spaces
+
+    private final ListProperty<BibtexStringEditorItemModel> stringsListProperty =
+            new SimpleListProperty<>(FXCollections.observableArrayList());
 
-    private final ListProperty<BibtexStringViewModel> allStrings = new SimpleListProperty<>(FXCollections.observableArrayList());
-    private final ObjectProperty<BibtexStringViewModel> selectedItemProperty = new SimpleObjectProperty<>();
     private final BibDatabase bibDatabase;
     private final BooleanProperty validProperty = new SimpleBooleanProperty();
 
     public BibtexStringEditorDialogViewModel(BibDatabase bibDatabase) {
         this.bibDatabase = bibDatabase;
         addAllStringsFromDB();
 
-        ObservableList<ObservableValue<Boolean>> allValidProperty = EasyBind.map(allStringsProperty(), BibtexStringViewModel::combinedValidationValidProperty);
+        ObservableList<ObservableValue<Boolean>> allValidProperty =
+                EasyBind.map(stringsListProperty(), BibtexStringEditorItemModel::combinedValidationValidProperty);
         validProperty.bind(EasyBind.combine(allValidProperty, stream -> stream.allMatch(valid -> valid)));
     }
 
     private void addAllStringsFromDB() {
-
-        Set<BibtexStringViewModel> strings = bibDatabase.getStringValues()
-                                                        .stream()
-                                                        .sorted(new BibtexStringComparator(false))
-                                                        .map(this::convertFromBibTexString)
-                                                        .collect(Collectors.toSet());
-        allStrings.addAll(strings);
-    }
-
-    public ListProperty<BibtexStringViewModel> allStringsProperty() {
-        return this.allStrings;
+        stringsListProperty.addAll(bibDatabase.getStringValues().stream()
+                                              .sorted(new BibtexStringComparator(false))
+                                              .map(this::convertFromBibTexString)
+                                              .collect(Collectors.toSet()));
     }
 
     public void addNewString() {
-        allStrings.add(new BibtexStringViewModel("", ""));
+        BibtexStringEditorItemModel newItem;
+        if (labelAlreadyExists(NEW_STRING_LABEL).isPresent()) {
+            int i = 1;
+            while (labelAlreadyExists(NEW_STRING_LABEL + i).isPresent()) {
+                i++;
+            }
+            newItem = new BibtexStringEditorItemModel(NEW_STRING_LABEL + i, "");
+        } else {
+            newItem = new BibtexStringEditorItemModel(NEW_STRING_LABEL, "");
+        }
+
+        stringsListProperty.add(newItem);
     }
 
-    public void removeString() {
-        BibtexStringViewModel toBeRemoved = selectedItemProperty.getValue();
-        allStrings.remove(toBeRemoved);
+    public void removeString(BibtexStringEditorItemModel item) {
+        stringsListProperty.remove(item);
     }
 
-    private BibtexStringViewModel convertFromBibTexString(BibtexString bibtexString) {
-        return new BibtexStringViewModel(bibtexString.getName(), bibtexString.getContent());
-    }
-
-    public ObjectProperty<BibtexStringViewModel> seletedItemProperty() {
-        return this.selectedItemProperty;
+    private BibtexStringEditorItemModel convertFromBibTexString(BibtexString bibtexString) {
+        return new BibtexStringEditorItemModel(bibtexString.getName(), bibtexString.getContent());
     }
 
     public void save() {
-        List<BibtexString> stringsToAdd = allStrings.stream().map(this::fromBibtexStringViewModel).collect(Collectors.toList());
-        bibDatabase.setStrings(stringsToAdd);
+        bibDatabase.setStrings(stringsListProperty.stream()
+                                                  .map(this::fromBibtexStringViewModel)
+                                                  .collect(Collectors.toList()));
     }
 
-    private BibtexString fromBibtexStringViewModel(BibtexStringViewModel viewModel) {
+    private BibtexString fromBibtexStringViewModel(BibtexStringEditorItemModel viewModel) {
         String label = viewModel.labelProperty().getValue();
         String content = viewModel.contentProperty().getValue();
         return new BibtexString(label, content);
     }
 
-    public BooleanProperty validProperty() {
-        return validProperty;
+    public Optional<BibtexStringEditorItemModel> labelAlreadyExists(String label) {
+        return stringsListProperty.stream()
+                                  .filter(item -> item.labelProperty().getValue().equals(label))
+                                  .findFirst();
     }
 
     public void openHelpPage() {
         HelpAction.openHelpPage(HelpFile.STRING_EDITOR);
     }
+
+    public ListProperty<BibtexStringEditorItemModel> stringsListProperty() {
+        return stringsListProperty;
+    }
+
+    public BooleanProperty validProperty() {
+        return validProperty;
+    }
 }