Visual improvements to LinkedFilesEditor (#9114)

* Visual improvements to LinkedFilesEditor * CHANGELOG.md * Moved and changed icons for better readability * Updated CHANGELOG.md
JabRef · Sep 2, 2022 · fb80915 · fb80915
1 parent e0d735e
commit fb80915
Show file tree

Hide file tree

Showing 8 changed files with 106 additions and 59 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -34,6 +34,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - The [HtmlToLaTeXFormatter](https://docs.jabref.org/finding-sorting-and-cleaning-entries/saveactions#html-to-latex) keeps single `<` characters.
 - We fixed a performance regression when opening large libraries [#9041](https://github.com/JabRef/jabref/issues/9041)
 - We fixed a bug where spaces are trimmed when highlighting differences in the Entries merge dialog. [koppor#371](https://github.com/koppor/jabref/issues/371)
+- We fixed some visual glitches with the linked files editor field in the entry editor and increased its height. [#8823](https://github.com/JabRef/jabref/issues/8823)
 - We fixed several bugs regarding the manual and the autosave of library files that sometimes lead to exceptions or data loss. [#9067](https://github.com/JabRef/jabref/pull/9067), [#8448](https://github.com/JabRef/jabref/issues/8484), [#8746](https://github.com/JabRef/jabref/issues/8746), [#6684](https://github.com/JabRef/jabref/issues/6684), [#6644](https://github.com/JabRef/jabref/issues/6644), [#6102](https://github.com/JabRef/jabref/issues/6102), [#6002](https://github.com/JabRef/jabref/issues/6000)
 - We fixed an issue where applied save actions on saving the library file would lead to the dialog "The libary has been modified by another program" popping up [#4877](https://github.com/JabRef/jabref/issues/4877)
 - We fixed an issue where title case didn't capitalize words after en-dash characters [#9068]

diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java
@@ -16,11 +16,14 @@
 import javax.net.ssl.SSLSocketFactory;
 
 import javafx.beans.Observable;
+import javafx.beans.binding.Bindings;
+import javafx.beans.binding.ObjectBinding;
 import javafx.beans.property.BooleanProperty;
 import javafx.beans.property.DoubleProperty;
 import javafx.beans.property.SimpleBooleanProperty;
 import javafx.beans.property.SimpleDoubleProperty;
 import javafx.beans.property.StringProperty;
+import javafx.scene.Node;
 import javafx.scene.control.Alert.AlertType;
 import javafx.scene.control.ButtonBar.ButtonData;
 import javafx.scene.control.ButtonType;
@@ -83,6 +86,8 @@ public class LinkedFileViewModel extends AbstractViewModel {
     private final PreferencesService preferences;
     private final LinkedFileHandler linkedFileHandler;
 
+    private ObjectBinding<Node> linkedFileIconBinding;
+
     private final Validator fileExistsValidator;
 
     public LinkedFileViewModel(LinkedFile linkedFile,
@@ -178,6 +183,14 @@ public JabRefIcon getTypeIcon() {
                                 .orElse(IconTheme.JabRefIcons.FILE);
     }
 
+    public ObjectBinding<Node> typeIconProperty() {
+        if (linkedFileIconBinding == null) {
+            linkedFileIconBinding = Bindings.createObjectBinding(() -> this.getTypeIcon().getGraphicNode(), linkedFile.fileTypeProperty());
+        }
+
+        return linkedFileIconBinding;
+    }
+
     public void markAsAutomaticallyFound() {
         isAutomaticallyFound.setValue(true);
     }

diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.fxml b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.fxml
@@ -6,40 +6,40 @@
 <?import javafx.scene.control.Tooltip?>
 <?import javafx.scene.layout.HBox?>
 <?import javafx.scene.layout.StackPane?>
+<?import javafx.scene.layout.VBox?>
 <?import org.jabref.gui.icon.JabRefIconView?>
 <fx:root xmlns:fx="http://javafx.com/fxml/1" type="HBox" xmlns="http://javafx.com/javafx/8.0.112"
          fx:controller="org.jabref.gui.fieldeditors.LinkedFilesEditor">
     <ListView fx:id="listView" prefHeight="0" HBox.hgrow="ALWAYS" maxHeight="100" />
-    <Button onAction="#addNewFile"
-            styleClass="icon-button">
-        <graphic>
-            <JabRefIconView glyph="NEW_FILE"/>
-        </graphic>
-        <tooltip>
-            <Tooltip text="%Open"/>
-        </tooltip>
-    </Button>
-    <Button onAction="#fetchFulltext" styleClass="icon-button">
-        <graphic>
-            <StackPane>
-                <JabRefIconView glyph="FETCH_FULLTEXT"
-                                visible="${controller.viewModel.fulltextLookupInProgress == false}"/>
-                <ProgressIndicator maxHeight="12.0" maxWidth="12.0"
-                                   visible="${controller.viewModel.fulltextLookupInProgress}"/>
-            </StackPane>
-        </graphic>
-        <tooltip>
-            <Tooltip text="%Get fulltext"/>
-        </tooltip>
-    </Button>
-    <Button
-            onAction="#addFromURL"
-            styleClass="icon-button">
-        <graphic>
-            <JabRefIconView glyph="DOWNLOAD"/>
-        </graphic>
-        <tooltip>
-            <Tooltip text="%Download from URL"/>
-        </tooltip>
-    </Button>
+    <VBox>
+        <Button onAction="#addNewFile" styleClass="icon-button">
+            <graphic>
+                <JabRefIconView glyph="LINKED_FILE_ADD"/>
+            </graphic>
+            <tooltip>
+                <Tooltip text="%Open"/>
+            </tooltip>
+        </Button>
+        <Button onAction="#fetchFulltext" styleClass="icon-button">
+            <graphic>
+                <StackPane>
+                    <JabRefIconView glyph="FETCH_FULLTEXT"
+                                    visible="${controller.viewModel.fulltextLookupInProgress == false}"/>
+                    <ProgressIndicator maxHeight="12.0" maxWidth="12.0"
+                                       visible="${controller.viewModel.fulltextLookupInProgress}"/>
+                </StackPane>
+            </graphic>
+            <tooltip>
+                <Tooltip text="%Get fulltext"/>
+            </tooltip>
+        </Button>
+        <Button onAction="#addFromURL" styleClass="icon-button">
+            <graphic>
+                <JabRefIconView glyph="DOWNLOAD"/>
+            </graphic>
+            <tooltip>
+                <Tooltip text="%Download from URL"/>
+            </tooltip>
+        </Button>
+    </VBox>
 </fx:root>
diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java
@@ -12,7 +12,9 @@
 import javafx.scene.Parent;
 import javafx.scene.control.Button;
 import javafx.scene.control.ContextMenu;
+import javafx.scene.control.Label;
 import javafx.scene.control.ListView;
+import javafx.scene.control.OverrunStyle;
 import javafx.scene.control.ProgressBar;
 import javafx.scene.control.SelectionMode;
 import javafx.scene.control.SeparatorMenuItem;
@@ -84,7 +86,7 @@ public LinkedFilesEditor(Field field,
                   .load();
 
         ViewModelListCellFactory<LinkedFileViewModel> cellFactory = new ViewModelListCellFactory<LinkedFileViewModel>()
-                .withStringTooltip(LinkedFileViewModel::getDescription)
+                .withStringTooltip(LinkedFileViewModel::getDescriptionAndLink)
                 .withGraphic(this::createFileDisplay)
                 .withContextMenu(this::createContextMenuForFile)
                 .withOnMouseClickedEvent(this::handleItemMouseClick)
@@ -165,10 +167,17 @@ private Node createFileDisplay(LinkedFileViewModel linkedFile) {
         progressIndicator.progressProperty().bind(linkedFile.downloadProgressProperty());
         progressIndicator.visibleProperty().bind(linkedFile.downloadOngoingProperty());
 
+        Label label = new Label();
+        label.graphicProperty().bind(linkedFile.typeIconProperty());
+        label.textProperty().bind(linkedFile.linkProperty());
+        label.getStyleClass().setAll("file-row-text");
+        label.textOverrunProperty().setValue(OverrunStyle.LEADING_ELLIPSIS);
+        EasyBind.subscribe(linkedFile.isAutomaticallyFoundProperty(), found -> label.pseudoClassStateChanged(opacity, found));
+
         HBox info = new HBox(8);
         HBox.setHgrow(info, Priority.ALWAYS);
         info.setStyle("-fx-padding: 0.5em 0 0.5em 0;"); // To align with buttons below which also have 0.5em padding
-        info.getChildren().setAll(icon, link, desc, progressIndicator);
+        info.getChildren().setAll(label, progressIndicator);
 
         Button acceptAutoLinkedFile = IconTheme.JabRefIcons.AUTO_LINKED_FILE.asButton();
         acceptAutoLinkedFile.setTooltip(new Tooltip(Localization.lang("This file was found automatically. Do you want to link it to this entry?")));
@@ -177,7 +186,7 @@ private Node createFileDisplay(LinkedFileViewModel linkedFile) {
         acceptAutoLinkedFile.setOnAction(event -> linkedFile.acceptAsLinked());
         acceptAutoLinkedFile.getStyleClass().setAll("icon-button");
 
-        Button writeMetadataToPdf = IconTheme.JabRefIcons.IMPORT.asButton();
+        Button writeMetadataToPdf = IconTheme.JabRefIcons.PDF_METADATA_WRITE.asButton();
         writeMetadataToPdf.setTooltip(new Tooltip(Localization.lang("Write BibTeXEntry metadata to PDF.")));
         writeMetadataToPdf.visibleProperty().bind(linkedFile.isOfflinePdfProperty());
         writeMetadataToPdf.getStyleClass().setAll("icon-button");
@@ -186,7 +195,7 @@ private Node createFileDisplay(LinkedFileViewModel linkedFile) {
         writeMetadataToPdf.disableProperty().bind(writeMetadataToPdfCommand.executableProperty().not());
         writeMetadataToPdf.setOnAction(event -> writeMetadataToPdfCommand.execute());
 
-        Button parsePdfMetadata = IconTheme.JabRefIcons.FILE_SEARCH.asButton();
+        Button parsePdfMetadata = IconTheme.JabRefIcons.PDF_METADATA_READ.asButton();
         parsePdfMetadata.setTooltip(new Tooltip(Localization.lang("Parse Metadata from PDF.")));
         parsePdfMetadata.visibleProperty().bind(linkedFile.isOfflinePdfProperty());
         parsePdfMetadata.setOnAction(event -> {
@@ -195,9 +204,9 @@ private Node createFileDisplay(LinkedFileViewModel linkedFile) {
         });
         parsePdfMetadata.getStyleClass().setAll("icon-button");
 
-        HBox container = new HBox(10);
+        HBox container = new HBox(2);
         container.setPrefHeight(Double.NEGATIVE_INFINITY);
-
+        container.maxWidthProperty().bind(listView.widthProperty().subtract(20d));
         container.getChildren().addAll(acceptAutoLinkedFile, info, writeMetadataToPdf, parsePdfMetadata);
 
         return container;
@@ -261,7 +270,7 @@ private void handleItemMouseClick(LinkedFileViewModel linkedFile, MouseEvent eve
 
     @Override
     public double getWeight() {
-        return 2;
+        return 3;
     }
 
     private ContextMenu createContextMenuForFile(LinkedFileViewModel linkedFile) {

diff --git a/src/main/java/org/jabref/gui/icon/IconTheme.java b/src/main/java/org/jabref/gui/icon/IconTheme.java
@@ -192,6 +192,8 @@ public enum JabRefIcons implements JabRefIcon {
         DELETE_ENTRY(MaterialDesignD.DELETE),
         SEARCH(MaterialDesignM.MAGNIFY),
         FILE_SEARCH(MaterialDesignF.FILE_FIND),
+        PDF_METADATA_READ(MaterialDesignF.FORMAT_ALIGN_TOP),
+        PDF_METADATA_WRITE(MaterialDesignF.FORMAT_ALIGN_BOTTOM),
         ADVANCED_SEARCH(Color.CYAN, MaterialDesignM.MAGNIFY),
         PREFERENCES(MaterialDesignC.COG),
         SELECTORS(MaterialDesignS.STAR_SETTINGS),
@@ -309,6 +311,7 @@ public enum JabRefIcons implements JabRefIcon {
         NEW_GROUP(MaterialDesignP.PLUS),
         OPEN_LINK(MaterialDesignO.OPEN_IN_NEW),
         LOOKUP_IDENTIFIER(MaterialDesignS.SEARCH_WEB),
+        LINKED_FILE_ADD(MaterialDesignP.PLUS),
         FETCH_FULLTEXT(MaterialDesignS.SEARCH_WEB),
         FETCH_BY_IDENTIFIER(MaterialDesignC.CLIPBOARD_ARROW_DOWN),
         TOGGLE_ABBREVIATION(MaterialDesignF.FORMAT_ALIGN_CENTER),

diff --git a/src/main/java/org/jabref/gui/linkedfile/LinkedFileEditDialog.fxml b/src/main/java/org/jabref/gui/linkedfile/LinkedFileEditDialog.fxml
@@ -1,42 +1,52 @@
 <?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.DialogPane?>
 <?import javafx.scene.control.Label?>
 <?import javafx.scene.control.TextField?>
+<?import javafx.scene.control.Tooltip?>
 <?import javafx.scene.layout.ColumnConstraints?>
 <?import javafx.scene.layout.GridPane?>
 <?import javafx.scene.layout.RowConstraints?>
-<DialogPane minHeight="-Infinity" prefHeight="150.0" prefWidth="536.0" xmlns="http://javafx.com/javafx/8.0.141" xmlns:fx="http://javafx.com/fxml/1" fx:controller="org.jabref.gui.linkedfile.LinkedFileEditDialogView">
+<?import org.jabref.gui.icon.JabRefIconView?>
+<DialogPane minHeight="140.0" minWidth="550.0"
+            xmlns="http://javafx.com/javafx/8.0.141" xmlns:fx="http://javafx.com/fxml/1"
+            fx:controller="org.jabref.gui.linkedfile.LinkedFileEditDialogView">
     <content>
-        <GridPane>
+        <GridPane vgap="4" hgap="4">
             <columnConstraints>
                 <ColumnConstraints hgrow="NEVER" minWidth="10.0" prefWidth="100.0"/>
-                <ColumnConstraints hgrow="ALWAYS" minWidth="10.0" prefWidth="200.0"/>
-                <ColumnConstraints hgrow="SOMETIMES" minWidth="10.0" prefWidth="108.0"/>
+                <ColumnConstraints hgrow="ALWAYS" minWidth="10.0"/>
+                <ColumnConstraints hgrow="SOMETIMES" minWidth="10.0"/>
             </columnConstraints>
             <rowConstraints>
-                <RowConstraints minHeight="10.0" prefHeight="30.0" vgrow="SOMETIMES"/>
-                <RowConstraints minHeight="10.0" prefHeight="30.0" vgrow="SOMETIMES"/>
-                <RowConstraints minHeight="10.0" prefHeight="30.0" vgrow="SOMETIMES"/>
+                <RowConstraints/>
+                <RowConstraints/>
+                <RowConstraints/>
             </rowConstraints>
-            <Label minWidth="95.0" text="%Link"/>
-            <TextField fx:id="link" minHeight="-Infinity" minWidth="-Infinity" prefHeight="25.0" prefWidth="150.0"
+
+            <Label text="%Link"/>
+            <TextField fx:id="link" prefHeight="25.0"
                        GridPane.columnIndex="1" GridPane.hgrow="ALWAYS"/>
-            <Label text="Description" GridPane.rowIndex="1"/>
-            <TextField fx:id="description" minHeight="-Infinity" minWidth="-Infinity" prefHeight="25.0"
-                       prefWidth="150.0" GridPane.columnIndex="1" GridPane.hgrow="ALWAYS" GridPane.rowIndex="1"/>
-            <ComboBox fx:id="fileType" minHeight="-Infinity" prefWidth="150.0" GridPane.columnIndex="1"
-                      GridPane.columnSpan="2" GridPane.hgrow="ALWAYS" GridPane.rowIndex="2"/>
-            <Button id="browse" mnemonicParsing="false" onAction="#openBrowseDialog" text="%Browse"
-                    GridPane.columnIndex="2">
-                <GridPane.margin>
-                    <Insets left="10.0"/>
-                </GridPane.margin>
+            <Button id="browse" onAction="#openBrowseDialog"
+                    styleClass="icon-button,narrow"
+                    prefHeight="20.0" prefWidth="20.0" GridPane.columnIndex="2">
+                <graphic>
+                    <JabRefIconView glyph="OPEN"/>
+                </graphic>
+                <tooltip>
+                    <Tooltip text="%Browse"/>
+                </tooltip>
             </Button>
+
+            <Label text="Description" GridPane.rowIndex="1"/>
+            <TextField fx:id="description" prefHeight="25.0"
+                       GridPane.columnIndex="1" GridPane.hgrow="ALWAYS" GridPane.rowIndex="1"/>
+
             <Label text="Filetype" GridPane.rowIndex="2"/>
+            <ComboBox fx:id="fileType"
+                      GridPane.columnIndex="1" GridPane.hgrow="ALWAYS" GridPane.rowIndex="2"/>
         </GridPane>
     </content>
 </DialogPane>
diff --git a/src/main/java/org/jabref/gui/linkedfile/LinkedFileEditDialogView.java b/src/main/java/org/jabref/gui/linkedfile/LinkedFileEditDialogView.java
@@ -12,6 +12,8 @@
 import org.jabref.gui.StateManager;
 import org.jabref.gui.externalfiletype.ExternalFileType;
 import org.jabref.gui.util.BaseDialog;
+import org.jabref.gui.util.ViewModelListCellFactory;
+import org.jabref.logic.l10n.Localization;
 import org.jabref.model.entry.LinkedFile;
 import org.jabref.preferences.PreferencesService;
 
@@ -40,6 +42,8 @@ public LinkedFileEditDialogView(LinkedFile linkedFile) {
                   .setAsContent(this.getDialogPane());
 
         this.getDialogPane().getButtonTypes().addAll(ButtonType.APPLY, ButtonType.CANCEL);
+        this.setResizable(false);
+        this.setTitle(Localization.lang("Edit file link"));
 
         this.setResultConverter(button -> {
             if (button == ButtonType.APPLY) {
@@ -54,6 +58,11 @@ public LinkedFileEditDialogView(LinkedFile linkedFile) {
     private void initialize() {
         viewModel = new LinkedFilesEditDialogViewModel(linkedFile, stateManager.getActiveDatabase().get(), dialogService, preferences.getFilePreferences());
         fileType.itemsProperty().bindBidirectional(viewModel.externalFileTypeProperty());
+        new ViewModelListCellFactory<ExternalFileType>()
+                .withIcon(ExternalFileType::getIcon)
+                .withText(ExternalFileType::getName)
+                .install(fileType);
+
         description.textProperty().bindBidirectional(viewModel.descriptionProperty());
         link.textProperty().bindBidirectional(viewModel.linkProperty());
         fileType.valueProperty().bindBidirectional(viewModel.selectedExternalFileTypeProperty());

diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties
@@ -2514,6 +2514,8 @@ Plain\ Text=Plain Text
 Show\ Diff=Show Diff
 Merged\ Entry=Merged Entry
 
+Edit\ file\ link=Edit file link
+
 (Note\:\ If\ original\ entries\ lack\ keywords\ to\ qualify\ for\ the\ new\ group\ configuration,\ confirming\ here\ will\ add\ them)=(Note: If original entries lack keywords to qualify for the new group configuration, confirming here will add them)
 Assign=Assign
 Do\ not\ assign=Do not assign