Split JabRefFrame to View and ViewModel (#11180)

* Generify closeTabs * Introduce JabRefFrameViewModel * Simplify close methods * Move handleUiCommands to viewmodel * Inject variables by constructor * Move related classes to distinct package * Move entrytype dialog to distinct package * Remove unused Dialog class * Move SendAsEMailActions to frame package * Fix Javadoc * Fix stylesheet location * Fix ide warning * Fix JavaDoc * Reword * Replace listener workaround with simple call * Remove superfluous controlsfx DecorationPane and window sizes. * Rearrange methods to match interface * Fix updating SplitPaneDivider * Fix annotations and javadoc * Restore package private modifier * Move Telemetry classes to distinct package --------- Co-authored-by: Oliver Kopp <[email protected]>
JabRef · Apr 16, 2024 · a52d4b7 · a52d4b7
1 parent 54de527
commit a52d4b7
Show file tree

Hide file tree

Showing 39 changed files with 726 additions and 617 deletions.
diff --git a/src/main/java/org/jabref/gui/Dialog.java b/src/main/java/org/jabref/gui/Dialog.java
diff --git a/src/main/java/org/jabref/gui/DragAndDropHelper.java b/src/main/java/org/jabref/gui/DragAndDropHelper.java
diff --git a/src/main/java/org/jabref/gui/Globals.java b/src/main/java/org/jabref/gui/Globals.java
@@ -3,6 +3,7 @@
 import org.jabref.architecture.AllowedToUseAwt;
 import org.jabref.gui.keyboard.KeyBindingRepository;
 import org.jabref.gui.remote.CLIMessageHandler;
+import org.jabref.gui.telemetry.Telemetry;
 import org.jabref.gui.undo.CountingUndoManager;
 import org.jabref.gui.util.DefaultFileUpdateMonitor;
 import org.jabref.gui.util.DefaultTaskExecutor;

diff --git a/src/main/java/org/jabref/gui/JabRefGUI.java b/src/main/java/org/jabref/gui/JabRefGUI.java
@@ -11,10 +11,12 @@
 import javafx.stage.Stage;
 import javafx.stage.WindowEvent;
 
+import org.jabref.gui.frame.JabRefFrame;
 import org.jabref.gui.help.VersionWorker;
 import org.jabref.gui.icon.IconTheme;
 import org.jabref.gui.keyboard.TextInputKeyBindings;
 import org.jabref.gui.openoffice.OOBibBaseConnect;
+import org.jabref.gui.telemetry.Telemetry;
 import org.jabref.gui.theme.ThemeManager;
 import org.jabref.logic.UiCommand;
 import org.jabref.logic.l10n.Localization;
@@ -26,7 +28,6 @@
 import org.jabref.preferences.JabRefPreferences;
 
 import com.tobiasdiez.easybind.EasyBind;
-import impl.org.controlsfx.skin.DecorationPane;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -73,7 +74,11 @@ public void start(Stage stage) {
                 mainStage,
                 dialogService,
                 fileUpdateMonitor,
-                preferencesService);
+                preferencesService,
+                Globals.stateManager,
+                Globals.undoManager,
+                Globals.entryTypesManager,
+                Globals.TASK_EXECUTOR);
 
         openWindow();
 
@@ -154,12 +159,7 @@ private void openWindow() {
         }
         debugLogWindowState(mainStage);
 
-        // We create a decoration pane ourselves for performance reasons
-        // (otherwise it has to be injected later, leading to a complete redraw/relayout of the complete scene)
-        DecorationPane root = new DecorationPane();
-        root.getChildren().add(JabRefGUI.mainFrame);
-
-        Scene scene = new Scene(root, 800, 800);
+        Scene scene = new Scene(JabRefGUI.mainFrame);
         themeManager.installCss(scene);
 
         // Handle TextEditor key bindings
@@ -177,11 +177,15 @@ private void openWindow() {
     }
 
     public void onShowing(WindowEvent event) {
+        Platform.runLater(() -> mainFrame.updateDividerPosition());
+
         // Open last edited databases
         if (uiCommands.stream().noneMatch(UiCommand.BlankWorkspace.class::isInstance)
             && preferencesService.getWorkspacePreferences().shouldOpenLastEdited()) {
             mainFrame.openLastEditedDatabases();
         }
+
+        Telemetry.initTrackingNotification(dialogService, preferencesService.getTelemetryPreferences());
     }
 
     public void onCloseRequest(WindowEvent event) {

diff --git a/src/main/java/org/jabref/gui/LibraryTabContainer.java b/src/main/java/org/jabref/gui/LibraryTabContainer.java
@@ -4,6 +4,7 @@
 
 import org.jabref.model.database.BibDatabaseContext;
 
+import org.jspecify.annotations.NonNull;
 import org.jspecify.annotations.NullMarked;
 import org.jspecify.annotations.Nullable;
 
@@ -16,24 +17,19 @@ public interface LibraryTabContainer {
 
     void showLibraryTab(LibraryTab libraryTab);
 
-    void addTab(LibraryTab libraryTab, boolean raisePanel);
-
     void addTab(BibDatabaseContext bibDatabaseContext, boolean raisePanel);
 
+    void addTab(LibraryTab libraryTab, boolean raisePanel);
+
     /**
      * Closes a designated libraryTab
      *
-     * @param libraryTab to be closed.
+     * @param tab to be closed.
      * @return true if closing the tab was successful
      */
-    boolean closeTab(LibraryTab libraryTab);
+    boolean closeTab(@Nullable LibraryTab tab);
 
-    /**
-     * Closes the currently viewed libraryTab
-     *
-     * @return true if closing the tab was successful
-     */
-    boolean closeCurrentTab();
+    boolean closeTabs(@NonNull List<LibraryTab> tabs);
 
     /**
      * Refreshes the ui after changes to the preferences

diff --git a/src/main/java/org/jabref/gui/actions/JabRefAction.java b/src/main/java/org/jabref/gui/actions/JabRefAction.java
@@ -4,8 +4,8 @@
 
 import javafx.beans.binding.Bindings;
 
-import org.jabref.gui.Telemetry;
 import org.jabref.gui.keyboard.KeyBindingRepository;
+import org.jabref.gui.telemetry.Telemetry;
 
 import de.saxsys.mvvmfx.utils.commands.Command;
 

diff --git a/src/main/java/org/jabref/gui/bibtexextractor/BibtexExtractorViewModel.java b/src/main/java/org/jabref/gui/bibtexextractor/BibtexExtractorViewModel.java
@@ -11,8 +11,8 @@
 
 import org.jabref.gui.DialogService;
 import org.jabref.gui.StateManager;
-import org.jabref.gui.Telemetry;
 import org.jabref.gui.externalfiles.ImportHandler;
+import org.jabref.gui.telemetry.Telemetry;
 import org.jabref.gui.util.BackgroundTask;
 import org.jabref.gui.util.TaskExecutor;
 import org.jabref.logic.importer.FetcherException;

diff --git a/src/main/java/org/jabref/gui/EntryType.fxml → ...a/org/jabref/gui/entrytype/EntryType.fxml b/src/main/java/org/jabref/gui/EntryType.fxml → ...a/org/jabref/gui/entrytype/EntryType.fxml
@@ -13,7 +13,7 @@
 <?import javafx.scene.layout.RowConstraints?>
 <?import javafx.scene.layout.VBox?>
 <DialogPane xmlns:fx="http://javafx.com/fxml/1" prefWidth="476.0" xmlns="http://javafx.com/javafx/8.0.171"
-            fx:controller="org.jabref.gui.EntryTypeView">
+            fx:controller="org.jabref.gui.entrytype.EntryTypeView">
     <buttonTypes>
         <ButtonType fx:constant="CANCEL"/>
         <ButtonType fx:id="generateButton" buttonData="OK_DONE" text="%Generate"/>

diff --git a/...in/java/org/jabref/gui/EntryTypeView.java → ...g/jabref/gui/entrytype/EntryTypeView.java b/...in/java/org/jabref/gui/EntryTypeView.java → ...g/jabref/gui/entrytype/EntryTypeView.java
@@ -1,4 +1,4 @@
-package org.jabref.gui;
+package org.jabref.gui.entrytype;
 
 import java.util.Collection;
 import java.util.List;
@@ -17,6 +17,10 @@
 import javafx.scene.layout.FlowPane;
 import javafx.stage.Screen;
 
+import org.jabref.gui.DialogService;
+import org.jabref.gui.Globals;
+import org.jabref.gui.LibraryTab;
+import org.jabref.gui.StateManager;
 import org.jabref.gui.util.BaseDialog;
 import org.jabref.gui.util.ControlHelper;
 import org.jabref.gui.util.IconValidationDecorator;

diff --git a/...va/org/jabref/gui/EntryTypeViewModel.java → ...ref/gui/entrytype/EntryTypeViewModel.java b/...va/org/jabref/gui/EntryTypeViewModel.java → ...ref/gui/entrytype/EntryTypeViewModel.java
@@ -1,4 +1,4 @@
-package org.jabref.gui;
+package org.jabref.gui.entrytype;
 
 import java.util.Optional;
 
@@ -14,6 +14,9 @@
 import javafx.concurrent.Task;
 import javafx.concurrent.Worker;
 
+import org.jabref.gui.DialogService;
+import org.jabref.gui.LibraryTab;
+import org.jabref.gui.StateManager;
 import org.jabref.gui.externalfiles.ImportHandler;
 import org.jabref.gui.importer.NewEntryAction;
 import org.jabref.gui.util.TaskExecutor;

diff --git a/...org/jabref/gui/menus/FileHistoryMenu.java → ...org/jabref/gui/frame/FileHistoryMenu.java b/...org/jabref/gui/menus/FileHistoryMenu.java → ...org/jabref/gui/frame/FileHistoryMenu.java
@@ -1,4 +1,4 @@
-package org.jabref.gui.menus;
+package org.jabref.gui.frame;
 
 import java.nio.file.Files;
 import java.nio.file.Path;

diff --git a/.../java/org/jabref/gui/FrameDndHandler.java → ...org/jabref/gui/frame/FrameDndHandler.java b/.../java/org/jabref/gui/FrameDndHandler.java → ...org/jabref/gui/frame/FrameDndHandler.java
@@ -1,8 +1,11 @@
-package org.jabref.gui;
+package org.jabref.gui.frame;
 
+import java.io.File;
 import java.nio.file.Path;
+import java.util.Collections;
 import java.util.List;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
 
 import javafx.scene.Node;
 import javafx.scene.Scene;
@@ -13,8 +16,12 @@
 import javafx.scene.input.Dragboard;
 import javafx.scene.input.TransferMode;
 
+import org.jabref.gui.DragAndDropDataFormats;
+import org.jabref.gui.LibraryTab;
+import org.jabref.gui.StateManager;
 import org.jabref.gui.importer.actions.OpenDatabaseAction;
 import org.jabref.logic.l10n.Localization;
+import org.jabref.logic.util.io.FileUtil;
 import org.jabref.model.entry.BibEntry;
 import org.jabref.model.groups.GroupTreeNode;
 
@@ -53,7 +60,7 @@ void initDragAndDrop() {
                 // drag'n'drop on tabs covered dnd on tabbedPane, so dnd on tabs should contain all dnds on tabbedPane
                 for (Node destinationTabNode : tabPane.lookupAll(".tab")) {
                     destinationTabNode.setOnDragOver(tabDragEvent -> onTabDragOver(event, tabDragEvent, dndIndicator));
-                    destinationTabNode.setOnDragExited(event1 -> tabPane.getTabs().remove(dndIndicator));
+                    destinationTabNode.setOnDragExited(tabDragEvent -> tabPane.getTabs().remove(dndIndicator));
                     destinationTabNode.setOnDragDropped(tabDragEvent -> onTabDragDropped(destinationTabNode, tabDragEvent, dndIndicator));
                 }
                 event.consume();
@@ -63,12 +70,12 @@ void initDragAndDrop() {
         });
     }
 
-    void onTabDragDropped(Node destinationTabNode, DragEvent tabDragEvent, Tab dndIndicator) {
+    private void onTabDragDropped(Node destinationTabNode, DragEvent tabDragEvent, Tab dndIndicator) {
         Dragboard dragboard = tabDragEvent.getDragboard();
 
-        if (DragAndDropHelper.hasBibFiles(dragboard)) {
+        if (hasBibFiles(dragboard)) {
             tabPane.getTabs().remove(dndIndicator);
-            List<Path> bibFiles = DragAndDropHelper.getBibFiles(dragboard);
+            List<Path> bibFiles = getBibFiles(dragboard);
             OpenDatabaseAction openDatabaseAction = this.openDatabaseAction.get();
             openDatabaseAction.openFiles(bibFiles);
             tabDragEvent.setDropCompleted(true);
@@ -83,8 +90,8 @@ void onTabDragDropped(Node destinationTabNode, DragEvent tabDragEvent, Tab dndIn
                 if (libraryTab.getId().equals(destinationTabNode.getId()) &&
                         !tabPane.getSelectionModel().getSelectedItem().equals(libraryTab)) {
                     LibraryTab destinationLibraryTab = (LibraryTab) libraryTab;
-                    if (DragAndDropHelper.hasGroups(dragboard)) {
-                        List<String> groupPathToSources = DragAndDropHelper.getGroups(dragboard);
+                    if (hasGroups(dragboard)) {
+                        List<String> groupPathToSources = getGroups(dragboard);
 
                         copyRootNode(destinationLibraryTab);
 
@@ -113,8 +120,8 @@ void onTabDragDropped(Node destinationTabNode, DragEvent tabDragEvent, Tab dndIn
         }
     }
 
-    void onTabDragOver(DragEvent event, DragEvent tabDragEvent, Tab dndIndicator) {
-        if (DragAndDropHelper.hasBibFiles(tabDragEvent.getDragboard()) || DragAndDropHelper.hasGroups(tabDragEvent.getDragboard())) {
+    private void onTabDragOver(DragEvent event, DragEvent tabDragEvent, Tab dndIndicator) {
+        if (hasBibFiles(tabDragEvent.getDragboard()) || hasGroups(tabDragEvent.getDragboard())) {
             tabDragEvent.acceptTransferModes(TransferMode.ANY);
             if (!tabPane.getTabs().contains(dndIndicator)) {
                 tabPane.getTabs().add(dndIndicator);
@@ -130,8 +137,8 @@ void onTabDragOver(DragEvent event, DragEvent tabDragEvent, Tab dndIndicator) {
         }
     }
 
-    void onSceneDragOver(DragEvent event, Tab dndIndicator) {
-        if (DragAndDropHelper.hasBibFiles(event.getDragboard())) {
+    private void onSceneDragOver(DragEvent event, Tab dndIndicator) {
+        if (hasBibFiles(event.getDragboard())) {
             event.acceptTransferModes(TransferMode.ANY);
             if (!tabPane.getTabs().contains(dndIndicator)) {
                 tabPane.getTabs().add(dndIndicator);
@@ -149,7 +156,7 @@ void onSceneDragOver(DragEvent event, Tab dndIndicator) {
 
     private void onSceneDragDropped(DragEvent event, Tab dndIndicator) {
         tabPane.getTabs().remove(dndIndicator);
-        List<Path> bibFiles = DragAndDropHelper.getBibFiles(event.getDragboard());
+        List<Path> bibFiles = getBibFiles(event.getDragboard());
         OpenDatabaseAction openDatabaseAction = this.openDatabaseAction.get();
         openDatabaseAction.openFiles(bibFiles);
         event.setDropCompleted(true);
@@ -195,4 +202,28 @@ private void copyGroupTreeNode(LibraryTab destinationLibraryTab, GroupTreeNode p
             }
         }
     }
+
+    private boolean hasBibFiles(Dragboard dragboard) {
+        return !getBibFiles(dragboard).isEmpty();
+    }
+
+    private List<Path> getBibFiles(Dragboard dragboard) {
+        if (!dragboard.hasFiles()) {
+            return Collections.emptyList();
+        } else {
+            return dragboard.getFiles().stream().map(File::toPath).filter(FileUtil::isBibFile).collect(Collectors.toList());
+        }
+    }
+
+    private boolean hasGroups(Dragboard dragboard) {
+        return !getGroups(dragboard).isEmpty();
+    }
+
+    private List<String> getGroups(Dragboard dragboard) {
+        if (!dragboard.hasContent(DragAndDropDataFormats.GROUP)) {
+            return Collections.emptyList();
+        } else {
+            return (List<String>) dragboard.getContent(DragAndDropDataFormats.GROUP);
+        }
+    }
 }