From 13fc12203b47485c445feb0428e099ea9bcf021f Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sat, 30 Oct 2021 15:48:53 +0100 Subject: [PATCH 01/36] Add visibleProperty to SidePaneComponent --- .../jabref/gui/sidepane/SidePaneComponent.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java b/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java index 15e80c6ab68..6a039ebb081 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java @@ -3,6 +3,8 @@ import java.util.Collections; import java.util.List; +import javafx.beans.property.BooleanProperty; +import javafx.beans.property.SimpleBooleanProperty; import javafx.scene.Node; import javafx.scene.control.Button; import javafx.scene.control.Label; @@ -24,6 +26,7 @@ public abstract class SidePaneComponent { private final JabRefIcon icon; private final String title; private Node contentNode; + private final BooleanProperty visible = new SimpleBooleanProperty(); public SidePaneComponent(SidePane sidePane, JabRefIcon icon, String title) { this.sidePane = sidePane; @@ -32,11 +35,25 @@ public SidePaneComponent(SidePane sidePane, JabRefIcon icon, String title) { this.toggleCommand = new ToggleCommand(this); } + public BooleanProperty visibleProperty() { + return visible; + } + + public boolean isVisible() { + return visibleProperty().get(); + } + + // TODO ('hide() is only used in SidePane') protected void hide() { + visible.setValue(false); + // TODO('Model should not update the view directly') sidePane.hide(this.getType()); } + // TODO ('show() is only used in SidePane') protected void show() { + visible.setValue(true); + // TODO('Model should not update the view directly') sidePane.show(this.getType()); } @@ -96,6 +113,7 @@ public final Node getContentPane() { public final Node getHeader() { Button close = IconTheme.JabRefIcons.CLOSE.asButton(); close.setTooltip(new Tooltip(Localization.lang("Hide panel"))); + // TODO (Use visibleProperty inside onAction) close.setOnAction(event -> hide()); Button up = IconTheme.JabRefIcons.UP.asButton(); From 9688ec6693b1c35aa178e03afae9692037dc0890 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sun, 31 Oct 2021 01:35:40 +0100 Subject: [PATCH 02/36] Making progress into refactoring Sidepane logic - Add immutable info to SidePaneType enum - Replace SidePaneComponent by SidePaneView which has a SidePaneHeaderView that contains all input handling - Create SidePaneContainerView the replacement for SidePane --- .../sidepane/GroupsSidePaneHeaderView.java | 59 ++++++++++ .../gui/sidepane/GroupsSidePaneView.java | 17 +++ .../org/jabref/gui/sidepane/SidePane.java | 15 +++ .../gui/sidepane/SidePaneContainerView.java | 109 ++++++++++++++++++ .../sidepane/SidePaneContainerViewModel.java | 22 ++++ .../gui/sidepane/SidePaneHeaderView.java | 61 ++++++++++ .../org/jabref/gui/sidepane/SidePaneType.java | 32 ++++- .../org/jabref/gui/sidepane/SidePaneView.java | 34 ++++++ .../gui/sidepane/SidePaneViewModel.java | 4 + 9 files changed, 350 insertions(+), 3 deletions(-) create mode 100644 src/main/java/org/jabref/gui/sidepane/GroupsSidePaneHeaderView.java create mode 100644 src/main/java/org/jabref/gui/sidepane/GroupsSidePaneView.java create mode 100644 src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java create mode 100644 src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java create mode 100644 src/main/java/org/jabref/gui/sidepane/SidePaneHeaderView.java create mode 100644 src/main/java/org/jabref/gui/sidepane/SidePaneView.java create mode 100644 src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java diff --git a/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneHeaderView.java b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneHeaderView.java new file mode 100644 index 00000000000..112f6d4306b --- /dev/null +++ b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneHeaderView.java @@ -0,0 +1,59 @@ +package org.jabref.gui.sidepane; + +import javafx.scene.control.Button; + +import org.jabref.gui.DialogService; +import org.jabref.gui.Globals; +import org.jabref.gui.actions.ActionFactory; +import org.jabref.gui.actions.SimpleCommand; +import org.jabref.gui.groups.GroupModeViewModel; +import org.jabref.gui.groups.GroupViewMode; +import org.jabref.gui.icon.IconTheme; +import org.jabref.logic.l10n.Localization; +import org.jabref.preferences.PreferencesService; + +public class GroupsSidePaneHeaderView extends SidePaneHeaderView { + private final PreferencesService preferences; + private final DialogService dialogService; + private final Button intersectionUnionToggle = IconTheme.JabRefIcons.GROUP_INTERSECTION.asButton(); + + public GroupsSidePaneHeaderView(SidePaneType sidePaneType, SimpleCommand closeCommand, SimpleCommand moveUpCommand, SimpleCommand moveDownCommand, PreferencesService preferences, DialogService dialogService) { + super(sidePaneType, closeCommand, moveUpCommand, moveDownCommand); + this.preferences = preferences; + this.dialogService = dialogService; + setupIntersectionUnionToggle(); + } + + private void setupIntersectionUnionToggle() { + addButtonAtPosition(intersectionUnionToggle, 2); + ActionFactory factory = new ActionFactory(Globals.getKeyPrefs()); + factory.configureIconButton(getSidePaneType().getToggleAction(), new ToggleUnionIntersectionAction(), intersectionUnionToggle); + } + + public void afterOpening() { + setGraphicsAndTooltipForButton(preferences.getGroupViewMode()); + } + + private void setGraphicsAndTooltipForButton(GroupViewMode mode) { + GroupModeViewModel modeViewModel = new GroupModeViewModel(mode); + intersectionUnionToggle.setGraphic(modeViewModel.getUnionIntersectionGraphic()); + intersectionUnionToggle.setTooltip(modeViewModel.getUnionIntersectionTooltip()); + } + + public class ToggleUnionIntersectionAction extends SimpleCommand { + + @Override + public void execute() { + GroupViewMode mode = preferences.getGroupViewMode(); + + if (mode == GroupViewMode.UNION) { + preferences.setGroupViewMode(GroupViewMode.INTERSECTION); + dialogService.notify(Localization.lang("Group view mode set to intersection")); + } else if (mode == GroupViewMode.INTERSECTION) { + preferences.setGroupViewMode(GroupViewMode.UNION); + dialogService.notify(Localization.lang("Group view mode set to union")); + } + setGraphicsAndTooltipForButton(mode); + } + } +} diff --git a/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneView.java b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneView.java new file mode 100644 index 00000000000..e80fc9e1add --- /dev/null +++ b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneView.java @@ -0,0 +1,17 @@ +package org.jabref.gui.sidepane; + +import javafx.scene.Node; +import javafx.scene.layout.Priority; + +import org.jabref.gui.DialogService; +import org.jabref.preferences.PreferencesService; + +public class GroupsSidePaneView extends SidePaneView { + public GroupsSidePaneView(GroupsSidePaneHeaderView sidePaneHeaderView, Node content, Priority resizePolicy, PreferencesService preferences, DialogService dialogService) { + super(sidePaneHeaderView, content, resizePolicy); + } + + public void afterOpening() { + ((GroupsSidePaneHeaderView) (getSidePaneHeaderView())).afterOpening(); + } +} diff --git a/src/main/java/org/jabref/gui/sidepane/SidePane.java b/src/main/java/org/jabref/gui/sidepane/SidePane.java index ef565fbbf2d..ed01e072ee7 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePane.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePane.java @@ -45,6 +45,7 @@ public SidePane(PreferencesService preferencesService, this.dialogService = dialogService; this.stateManager = stateManager; this.undoManager = undoManager; + initComponents(); setId("sidePane"); @@ -53,7 +54,21 @@ public SidePane(PreferencesService preferencesService, updateView(); } + private void initComponents() { + components.put(SidePaneType.GROUPS, new GroupSidePane(this, taskExecutor, stateManager, preferencesService, dialogService)); + components.put(SidePaneType.OPEN_OFFICE, new OpenOfficeSidePanel(this, taskExecutor, preferencesService, dialogService, stateManager, undoManager)); + components.put(SidePaneType.WEB_SEARCH, new WebSearchPane(this, preferencesService, dialogService, stateManager)); + components.forEach((key, value) -> value.visibleProperty().addListener((x, y, isVisible) -> { + if (isVisible) { + show(key); + } else { + hide(key); + } + })); + } + public boolean isComponentVisible(SidePaneType type) { + // TODO('return getComponent(type).visiblePropertyProperty().get();') return visibleComponents.contains(getComponent(type)); } diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java new file mode 100644 index 00000000000..fef441d3886 --- /dev/null +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java @@ -0,0 +1,109 @@ +package org.jabref.gui.sidepane; + +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import javax.swing.undo.UndoManager; + +import javafx.collections.FXCollections; +import javafx.collections.ObservableList; +import javafx.scene.layout.Priority; +import javafx.scene.layout.VBox; + +import org.jabref.gui.DialogService; +import org.jabref.gui.StateManager; +import org.jabref.gui.actions.SimpleCommand; +import org.jabref.gui.groups.GroupTreeView; +import org.jabref.gui.util.TaskExecutor; +import org.jabref.preferences.PreferencesService; + +public class SidePaneContainerView extends VBox { + private Map sidePaneViewLookup; + // TODO('Use preferencesService.getSidePanePreferences().visiblePanes() as the single source of truth') + private final ObservableList visiblePanes = FXCollections.observableArrayList(); + + private final PreferencesService preferencesService; + private final TaskExecutor taskExecutor; + private final DialogService dialogService; + private final StateManager stateManager; + private final UndoManager undoManager; + + public SidePaneContainerView(PreferencesService preferencesService, + TaskExecutor taskExecutor, + DialogService dialogService, + StateManager stateManager, + UndoManager undoManager) { + this.preferencesService = preferencesService; + this.taskExecutor = taskExecutor; + this.dialogService = dialogService; + this.stateManager = stateManager; + this.undoManager = undoManager; + initSidePanes(); + } + + private void initSidePanes() { + SidePaneView groupsPaneView = createGroupsPaneView(); + } + + private SidePaneView createGroupsPaneView() { + GroupsSidePaneHeaderView groupsHeader = new GroupsSidePaneHeaderView(SidePaneType.GROUPS, + new CloseSidePaneAction(SidePaneType.GROUPS), null, null, preferencesService, dialogService); + GroupTreeView groupsContent = new GroupTreeView(taskExecutor, stateManager, preferencesService, dialogService); + + return new GroupsSidePaneView(groupsHeader, groupsContent, Priority.ALWAYS, preferencesService, dialogService); + } + + private void showSidePanes(Set toShowSidePanes) { + getChildren().clear(); + toShowSidePanes.forEach(type -> { + SidePaneView view = sidePaneViewLookup.get(type); + getChildren().add(view); + VBox.setVgrow(view, view.getResizePolicy()); + }); + } + + private void show(SidePaneType paneType) { + if (!visiblePanes.contains(paneType)) { + visiblePanes.add(paneType); + preferencesService.getSidePanePreferences().visiblePanes().add(paneType); + // TODO('Sort') + updateView(); + } + } + + private void hide(SidePaneType paneType) { + if (visiblePanes.contains(paneType)) { + // TODO('Before closing') + visiblePanes.remove(paneType); + preferencesService.getSidePanePreferences().visiblePanes().remove(paneType); + updateView(); + } + } + + private void moveUp(SidePaneType type) { + + } + + public ObservableList getVisiblePanes() { + return visiblePanes; + } + + private void updateView() { + showSidePanes(new HashSet<>(visiblePanes)); + setVisible(!visiblePanes.isEmpty()); + } + + private class CloseSidePaneAction extends SimpleCommand { + private final SidePaneType toClosePaneType; + + public CloseSidePaneAction(SidePaneType toClosePaneType) { + this.toClosePaneType = toClosePaneType; + } + + @Override + public void execute() { + hide(toClosePaneType); + } + } +} diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java new file mode 100644 index 00000000000..9a8ca93714b --- /dev/null +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java @@ -0,0 +1,22 @@ +package org.jabref.gui.sidepane; + +import org.jabref.gui.AbstractViewModel; + +public class SidePaneContainerViewModel extends AbstractViewModel { + + public void moveUp(SidePaneType sidePaneType) { + + } + + public void moveDown(SidePaneType sidePaneType) { + + } + + public void show(SidePaneType sidePaneType) { + + } + + public void hide(SidePaneType sidePaneType) { + + } +} diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneHeaderView.java b/src/main/java/org/jabref/gui/sidepane/SidePaneHeaderView.java new file mode 100644 index 00000000000..3271c9fd212 --- /dev/null +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneHeaderView.java @@ -0,0 +1,61 @@ +package org.jabref.gui.sidepane; + +import javafx.scene.control.Button; +import javafx.scene.control.Label; +import javafx.scene.control.Tooltip; +import javafx.scene.layout.BorderPane; +import javafx.scene.layout.HBox; + +import org.jabref.gui.Globals; +import org.jabref.gui.actions.ActionFactory; +import org.jabref.gui.actions.SimpleCommand; +import org.jabref.gui.icon.IconTheme; +import org.jabref.logic.l10n.Localization; + +public class SidePaneHeaderView extends BorderPane { + private final SidePaneType sidePaneType; + private final SimpleCommand closeCommand; + private final SimpleCommand moveUpCommand; + private final SimpleCommand moveDownCommand; + + private HBox buttonContainer; + + public SidePaneHeaderView(SidePaneType sidePaneType, SimpleCommand closeCommand, SimpleCommand moveUpCommand, SimpleCommand moveDownCommand) { + this.sidePaneType = sidePaneType; + this.closeCommand = closeCommand; + this.moveUpCommand = moveUpCommand; + this.moveDownCommand = moveDownCommand; + initView(); + } + + private void initView() { + ActionFactory factory = new ActionFactory(Globals.getKeyPrefs()); + Button closeButton = IconTheme.JabRefIcons.CLOSE.asButton(); + closeButton.setTooltip(new Tooltip(Localization.lang("Hide panel"))); + factory.configureIconButton(sidePaneType.getToggleAction(), closeCommand, closeButton); + + Button upButton = IconTheme.JabRefIcons.UP.asButton(); + upButton.setTooltip(new Tooltip(Localization.lang("Move panel up"))); + factory.configureIconButton(sidePaneType.getToggleAction(), moveUpCommand, upButton); + + Button downButton = IconTheme.JabRefIcons.DOWN.asButton(); + downButton.setTooltip(new Tooltip(Localization.lang("Move panel down"))); + factory.configureIconButton(sidePaneType.getToggleAction(), moveDownCommand, downButton); + + this.buttonContainer = new HBox(); + buttonContainer.getChildren().addAll(upButton, downButton, closeButton); + + Label label = new Label(sidePaneType.getTitle()); + setCenter(label); + setRight(buttonContainer); + getStyleClass().add("sidePaneComponentHeader"); + } + + protected void addButtonAtPosition(Button button, int position) { + this.buttonContainer.getChildren().set(position, button); + } + + public SidePaneType getSidePaneType() { + return sidePaneType; + } +} diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneType.java b/src/main/java/org/jabref/gui/sidepane/SidePaneType.java index 038e48feb3d..c35e32f5a5a 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneType.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneType.java @@ -1,10 +1,36 @@ package org.jabref.gui.sidepane; +import org.jabref.gui.actions.Action; +import org.jabref.gui.actions.StandardActions; +import org.jabref.gui.icon.JabRefIcon; + /** * Definition of all possible components in the side pane. */ public enum SidePaneType { - OPEN_OFFICE, - WEB_SEARCH, - GROUPS + OPEN_OFFICE("", null, StandardActions.TOOGLE_OO), + WEB_SEARCH("", null, StandardActions.TOGGLE_WEB_SEARCH), + GROUPS("", null, StandardActions.TOGGLE_GROUPS); + + private final String title; + private final JabRefIcon icon; + private final Action toggleAction; + + SidePaneType(String title, JabRefIcon icon, Action toggleAction) { + this.title = title; + this.icon = icon; + this.toggleAction = toggleAction; + } + + public String getTitle() { + return title; + } + + public JabRefIcon getIcon() { + return icon; + } + + public Action getToggleAction() { + return toggleAction; + } } diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneView.java b/src/main/java/org/jabref/gui/sidepane/SidePaneView.java new file mode 100644 index 00000000000..a3912f1c535 --- /dev/null +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneView.java @@ -0,0 +1,34 @@ +package org.jabref.gui.sidepane; + +import java.util.Optional; + +import javafx.scene.Node; +import javafx.scene.layout.BorderPane; +import javafx.scene.layout.Priority; + +public class SidePaneView extends BorderPane { + private final SidePaneHeaderView sidePaneHeaderView; + private final Node content; + private final Priority resizePolicy; + + public SidePaneView(SidePaneHeaderView sidePaneHeaderView, Node content, Priority resizePolicy) { + this.sidePaneHeaderView = sidePaneHeaderView; + this.content = content; + this.resizePolicy = resizePolicy; + initView(); + } + + private void initView() { + getStyleClass().add("sidePaneComponent"); + setTop(sidePaneHeaderView); + setCenter(content); + } + + public Priority getResizePolicy() { + return resizePolicy; + } + + protected SidePaneHeaderView getSidePaneHeaderView() { + return sidePaneHeaderView; + } +} diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java new file mode 100644 index 00000000000..2382073b37f --- /dev/null +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java @@ -0,0 +1,4 @@ +package org.jabref.gui.sidepane; + +public class SidePaneViewModel { +} From e7ddff651784ac8b3975ec2437d78a930673340d Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sun, 31 Oct 2021 16:14:55 +0100 Subject: [PATCH 03/36] Populate SidePaneType with information --- src/main/java/org/jabref/gui/sidepane/SidePaneType.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneType.java b/src/main/java/org/jabref/gui/sidepane/SidePaneType.java index c35e32f5a5a..494f936bbe9 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneType.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneType.java @@ -2,15 +2,17 @@ import org.jabref.gui.actions.Action; import org.jabref.gui.actions.StandardActions; +import org.jabref.gui.icon.IconTheme; import org.jabref.gui.icon.JabRefIcon; +import org.jabref.logic.l10n.Localization; /** * Definition of all possible components in the side pane. */ public enum SidePaneType { - OPEN_OFFICE("", null, StandardActions.TOOGLE_OO), - WEB_SEARCH("", null, StandardActions.TOGGLE_WEB_SEARCH), - GROUPS("", null, StandardActions.TOGGLE_GROUPS); + OPEN_OFFICE("OpenOffice/LibreOffice", IconTheme.JabRefIcons.FILE_OPENOFFICE, StandardActions.TOOGLE_OO), + WEB_SEARCH(Localization.lang("Web search"), IconTheme.JabRefIcons.WWW, StandardActions.TOGGLE_WEB_SEARCH), + GROUPS(Localization.lang("Groups"), IconTheme.JabRefIcons.TOGGLE_GROUPS, StandardActions.TOGGLE_GROUPS); private final String title; private final JabRefIcon icon; From eee68e7c24dd948be5023668cb05eabd8b46116d Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sun, 31 Oct 2021 18:17:44 +0100 Subject: [PATCH 04/36] Wrap the content of web search side pane into WebSearchPaneView --- .../importer/fetcher/WebSearchPaneView.java | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneView.java diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneView.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneView.java new file mode 100644 index 00000000000..5f4b25269c2 --- /dev/null +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneView.java @@ -0,0 +1,95 @@ +package org.jabref.gui.importer.fetcher; + +import javafx.css.PseudoClass; +import javafx.geometry.Pos; +import javafx.scene.control.Button; +import javafx.scene.control.ComboBox; +import javafx.scene.control.TextField; +import javafx.scene.control.Tooltip; +import javafx.scene.input.KeyCode; +import javafx.scene.layout.HBox; +import javafx.scene.layout.Priority; +import javafx.scene.layout.StackPane; +import javafx.scene.layout.VBox; + +import org.jabref.gui.DialogService; +import org.jabref.gui.StateManager; +import org.jabref.gui.actions.ActionFactory; +import org.jabref.gui.actions.StandardActions; +import org.jabref.gui.help.HelpAction; +import org.jabref.gui.search.SearchTextField; +import org.jabref.gui.util.ViewModelListCellFactory; +import org.jabref.logic.importer.SearchBasedFetcher; +import org.jabref.logic.l10n.Localization; +import org.jabref.preferences.PreferencesService; + +import com.tobiasdiez.easybind.EasyBind; + +public class WebSearchPaneView extends VBox { + + private static final PseudoClass QUERY_INVALID = PseudoClass.getPseudoClass("invalid"); + + private final WebSearchPaneViewModel viewModel; + private final PreferencesService preferences; + + public WebSearchPaneView(PreferencesService preferences, DialogService dialogService, StateManager stateManager) { + this.preferences = preferences; + this.viewModel = new WebSearchPaneViewModel(preferences, dialogService, stateManager); + initView(); + } + + private void initView() { + ComboBox fetchers = new ComboBox<>(); + new ViewModelListCellFactory() + .withText(SearchBasedFetcher::getName) + .install(fetchers); + fetchers.itemsProperty().bind(viewModel.fetchersProperty()); + fetchers.valueProperty().bindBidirectional(viewModel.selectedFetcherProperty()); + fetchers.setMaxWidth(Double.POSITIVE_INFINITY); + + // Create help button for currently selected fetcher + StackPane helpButtonContainer = new StackPane(); + ActionFactory factory = new ActionFactory(preferences.getKeyBindingRepository()); + EasyBind.subscribe(viewModel.selectedFetcherProperty(), fetcher -> { + if ((fetcher != null) && fetcher.getHelpPage().isPresent()) { + Button helpButton = factory.createIconButton(StandardActions.HELP, new HelpAction(fetcher.getHelpPage().get())); + helpButtonContainer.getChildren().setAll(helpButton); + } else { + helpButtonContainer.getChildren().clear(); + } + }); + HBox fetcherContainer = new HBox(fetchers, helpButtonContainer); + HBox.setHgrow(fetchers, Priority.ALWAYS); + + // Create text field for query input + TextField query = SearchTextField.create(); + query.getStyleClass().add("searchBar"); + + viewModel.queryProperty().bind(query.textProperty()); + EasyBind.subscribe(viewModel.queryValidationStatus().validProperty(), + valid -> { + if (!valid && viewModel.queryValidationStatus().getHighestMessage().isPresent()) { + query.setTooltip(new Tooltip(viewModel.queryValidationStatus().getHighestMessage().get().getMessage())); + query.pseudoClassStateChanged(QUERY_INVALID, true); + } else { + query.setTooltip(null); + query.pseudoClassStateChanged(QUERY_INVALID, false); + } + }); + + // Allows triggering search on pressing enter + query.setOnKeyPressed(event -> { + if (event.getCode() == KeyCode.ENTER) { + viewModel.search(); + } + }); + + // Create button that triggers search + Button search = new Button(Localization.lang("Search")); + search.setDefaultButton(false); + search.setOnAction(event -> viewModel.search()); + + setAlignment(Pos.CENTER); + getChildren().addAll(fetcherContainer, query, search); + } +} From 9c3a3148b27dae5d14c58fe9819082eba5974540 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sun, 31 Oct 2021 18:38:40 +0100 Subject: [PATCH 05/36] Revert changes to SidePane --- .../java/org/jabref/gui/sidepane/SidePane.java | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/main/java/org/jabref/gui/sidepane/SidePane.java b/src/main/java/org/jabref/gui/sidepane/SidePane.java index ed01e072ee7..ef565fbbf2d 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePane.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePane.java @@ -45,7 +45,6 @@ public SidePane(PreferencesService preferencesService, this.dialogService = dialogService; this.stateManager = stateManager; this.undoManager = undoManager; - initComponents(); setId("sidePane"); @@ -54,21 +53,7 @@ public SidePane(PreferencesService preferencesService, updateView(); } - private void initComponents() { - components.put(SidePaneType.GROUPS, new GroupSidePane(this, taskExecutor, stateManager, preferencesService, dialogService)); - components.put(SidePaneType.OPEN_OFFICE, new OpenOfficeSidePanel(this, taskExecutor, preferencesService, dialogService, stateManager, undoManager)); - components.put(SidePaneType.WEB_SEARCH, new WebSearchPane(this, preferencesService, dialogService, stateManager)); - components.forEach((key, value) -> value.visibleProperty().addListener((x, y, isVisible) -> { - if (isVisible) { - show(key); - } else { - hide(key); - } - })); - } - public boolean isComponentVisible(SidePaneType type) { - // TODO('return getComponent(type).visiblePropertyProperty().get();') return visibleComponents.contains(getComponent(type)); } From f59bb3fef7d75dc3983eaf56f9d85cbefd307203 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sun, 31 Oct 2021 18:41:37 +0100 Subject: [PATCH 06/36] Refactor SidePaneContainer - Move some state and logic to SidePaneContainerViewModel - Implement the moveUp and moveDown action - Implement lazy loading for SidePaneView - Expose some initial API for fixing #8115 --- .../gui/sidepane/SidePaneContainerView.java | 138 ++++++++++++++---- .../sidepane/SidePaneContainerViewModel.java | 104 ++++++++++++- .../org/jabref/gui/sidepane/SidePaneView.java | 2 - .../gui/sidepane/SidePaneViewModel.java | 3 + 4 files changed, 209 insertions(+), 38 deletions(-) diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java index fef441d3886..b8d366c83a8 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java @@ -1,13 +1,15 @@ package org.jabref.gui.sidepane; +import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; import javax.swing.undo.UndoManager; -import javafx.collections.FXCollections; -import javafx.collections.ObservableList; +import javafx.beans.property.BooleanProperty; +import javafx.beans.property.SimpleBooleanProperty; +import javafx.scene.Node; import javafx.scene.layout.Priority; import javafx.scene.layout.VBox; @@ -15,13 +17,19 @@ import org.jabref.gui.StateManager; import org.jabref.gui.actions.SimpleCommand; import org.jabref.gui.groups.GroupTreeView; +import org.jabref.gui.importer.fetcher.WebSearchPaneView; +import org.jabref.gui.openoffice.OpenOfficePanel; import org.jabref.gui.util.TaskExecutor; import org.jabref.preferences.PreferencesService; +import static org.jabref.gui.sidepane.SidePaneType.GROUPS; +import static org.jabref.gui.sidepane.SidePaneType.OPEN_OFFICE; +import static org.jabref.gui.sidepane.SidePaneType.WEB_SEARCH; + public class SidePaneContainerView extends VBox { - private Map sidePaneViewLookup; - // TODO('Use preferencesService.getSidePanePreferences().visiblePanes() as the single source of truth') - private final ObservableList visiblePanes = FXCollections.observableArrayList(); + // Don't use this map directly to lookup sidePaneViews, instead use getSidePaneView() for lazy loading + private final Map sidePaneViewLookup = new HashMap<>(); + private final SidePaneContainerViewModel viewModel; private final PreferencesService preferencesService; private final TaskExecutor taskExecutor; @@ -29,6 +37,11 @@ public class SidePaneContainerView extends VBox { private final StateManager stateManager; private final UndoManager undoManager; + // Probably should go into the view model + private final BooleanProperty groupsPaneVisible = new SimpleBooleanProperty(); + private final BooleanProperty openOfficePaneVisible = new SimpleBooleanProperty(); + private final BooleanProperty webSearchPaneVisible = new SimpleBooleanProperty(); + public SidePaneContainerView(PreferencesService preferencesService, TaskExecutor taskExecutor, DialogService dialogService, @@ -39,71 +52,132 @@ public SidePaneContainerView(PreferencesService preferencesService, this.dialogService = dialogService; this.stateManager = stateManager; this.undoManager = undoManager; - initSidePanes(); + this.viewModel = new SidePaneContainerViewModel(preferencesService); + + preferencesService.getSidePanePreferences().visiblePanes().forEach(this::show); + updateView(); } - private void initSidePanes() { - SidePaneView groupsPaneView = createGroupsPaneView(); + private SidePaneView getSidePaneView(SidePaneType sidePaneType) { + SidePaneView sidePaneView = sidePaneViewLookup.get(sidePaneType); + if (sidePaneView == null) { + sidePaneView = switch (sidePaneType) { + case GROUPS -> createGroupsSidePaneView(); + case WEB_SEARCH -> createWebSearchSidePaneView(); + case OPEN_OFFICE -> createOpenOfficeSidePaneView(); + }; + sidePaneViewLookup.put(sidePaneType, sidePaneView); + } + return sidePaneView; } - private SidePaneView createGroupsPaneView() { - GroupsSidePaneHeaderView groupsHeader = new GroupsSidePaneHeaderView(SidePaneType.GROUPS, - new CloseSidePaneAction(SidePaneType.GROUPS), null, null, preferencesService, dialogService); + private SidePaneView createGroupsSidePaneView() { + GroupsSidePaneHeaderView groupsHeader = new GroupsSidePaneHeaderView(GROUPS, + new CloseSidePaneAction(GROUPS), new MoveUpAction(GROUPS), new MoveDownAction(GROUPS), preferencesService, dialogService); GroupTreeView groupsContent = new GroupTreeView(taskExecutor, stateManager, preferencesService, dialogService); return new GroupsSidePaneView(groupsHeader, groupsContent, Priority.ALWAYS, preferencesService, dialogService); } + private SidePaneView createOpenOfficeSidePaneView() { + Node openOfficePaneContent = new OpenOfficePanel(preferencesService, preferencesService.getOpenOfficePreferences(), + preferencesService.getKeyBindingRepository(), taskExecutor, dialogService, stateManager, undoManager).getContent(); + return createSidePaneView(OPEN_OFFICE, openOfficePaneContent); + } + + private SidePaneView createWebSearchSidePaneView() { + Node webSearchPaneContent = new WebSearchPaneView(preferencesService, dialogService, stateManager); + return createSidePaneView(WEB_SEARCH, webSearchPaneContent); + } + + private SidePaneView createSidePaneView(SidePaneType sidePaneType, Node paneContent) { + SidePaneHeaderView paneHeaderView = new SidePaneHeaderView(sidePaneType, new CloseSidePaneAction(sidePaneType), + new MoveUpAction(sidePaneType), new MoveDownAction(sidePaneType)); + return new SidePaneView(paneHeaderView, paneContent, Priority.NEVER); + } + private void showSidePanes(Set toShowSidePanes) { getChildren().clear(); toShowSidePanes.forEach(type -> { - SidePaneView view = sidePaneViewLookup.get(type); + SidePaneView view = getSidePaneView(type); getChildren().add(view); VBox.setVgrow(view, view.getResizePolicy()); }); } - private void show(SidePaneType paneType) { - if (!visiblePanes.contains(paneType)) { - visiblePanes.add(paneType); - preferencesService.getSidePanePreferences().visiblePanes().add(paneType); - // TODO('Sort') + private void show(SidePaneType sidePane) { + if (viewModel.show(sidePane)) { updateView(); } } - private void hide(SidePaneType paneType) { - if (visiblePanes.contains(paneType)) { - // TODO('Before closing') - visiblePanes.remove(paneType); - preferencesService.getSidePanePreferences().visiblePanes().remove(paneType); + private void hide(SidePaneType sidePane) { + if (viewModel.hide(sidePane)) { updateView(); } } - private void moveUp(SidePaneType type) { - + private void moveUp(SidePaneType sidePane) { + if (viewModel.moveUp(sidePane)) { + updateView(); + } } - public ObservableList getVisiblePanes() { - return visiblePanes; + private void moveDown(SidePaneType sidePane) { + if (viewModel.moveDown(sidePane)) { + updateView(); + } } private void updateView() { - showSidePanes(new HashSet<>(visiblePanes)); - setVisible(!visiblePanes.isEmpty()); + showSidePanes(new HashSet<>(viewModel.getVisiblePanes())); + setVisible(!viewModel.getVisiblePanes().isEmpty()); + } + + public BooleanProperty isPaneVisibleProperty(SidePaneType sidePaneType) { + return switch (sidePaneType) { + case GROUPS -> groupsPaneVisible; + case WEB_SEARCH -> webSearchPaneVisible; + case OPEN_OFFICE -> openOfficePaneVisible; + }; } private class CloseSidePaneAction extends SimpleCommand { - private final SidePaneType toClosePaneType; + private final SidePaneType toCloseSidePane; + + public CloseSidePaneAction(SidePaneType toCloseSidePane) { + this.toCloseSidePane = toCloseSidePane; + } + + @Override + public void execute() { + hide(toCloseSidePane); + } + } + + private class MoveUpAction extends SimpleCommand { + private final SidePaneType toMoveUpSidePane; + + public MoveUpAction(SidePaneType toMoveUpSidePane) { + this.toMoveUpSidePane = toMoveUpSidePane; + } + + @Override + public void execute() { + moveUp(toMoveUpSidePane); + } + } + + private class MoveDownAction extends SimpleCommand { + private final SidePaneType toMoveDownSidePane; - public CloseSidePaneAction(SidePaneType toClosePaneType) { - this.toClosePaneType = toClosePaneType; + public MoveDownAction(SidePaneType toMoveDownSidePane) { + this.toMoveDownSidePane = toMoveDownSidePane; } @Override public void execute() { - hide(toClosePaneType); + moveDown(toMoveDownSidePane); } } } diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java index 9a8ca93714b..f1e8dbb7086 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java @@ -1,22 +1,118 @@ package org.jabref.gui.sidepane; +import java.util.Comparator; +import java.util.HashMap; +import java.util.Map; +import java.util.stream.IntStream; + +import javafx.collections.FXCollections; +import javafx.collections.ObservableList; + import org.jabref.gui.AbstractViewModel; +import org.jabref.preferences.PreferencesService; public class SidePaneContainerViewModel extends AbstractViewModel { + private final PreferencesService preferencesService; + // TODO('Use preferencesService.getSidePanePreferences().visiblePanes() as the single source of truth') + private final ObservableList visiblePanes = FXCollections.observableArrayList(); + + public SidePaneContainerViewModel(PreferencesService preferencesService) { + this.preferencesService = preferencesService; + } - public void moveUp(SidePaneType sidePaneType) { + /** + * Stores the current configuration of visible panes in the preferences, + * so that we show panes at the preferred position next time. + */ + private void updatePreferredPositions() { + Map preferredPositions = new HashMap<>(preferencesService.getSidePanePreferences().getPreferredPositions()); + IntStream.range(0, visiblePanes.size()).forEach(i -> preferredPositions.put(visiblePanes.get(i), i)); + preferencesService.getSidePanePreferences().setPreferredPositions(preferredPositions); + } + public ObservableList getVisiblePanes() { + return visiblePanes; } - public void moveDown(SidePaneType sidePaneType) { + /** + * @return True if sidePane is visible, and it can still move up therefore we should update the view + */ + public boolean moveUp(SidePaneType sidePane) { + if (visiblePanes.contains(sidePane)) { + int currentPosition = visiblePanes.indexOf(sidePane); + if (currentPosition > 0) { + int newPosition = currentPosition - 1; + visiblePanes.remove(currentPosition); + visiblePanes.add(newPosition, sidePane); + updatePreferredPositions(); + return true; + } + } + return false; } - public void show(SidePaneType sidePaneType) { + /** + * @return True if sidePane is visible, and it can still move down therefore we should update the view + */ + public boolean moveDown(SidePaneType sidePane) { + if (visiblePanes.contains(sidePane)) { + int currentPosition = visiblePanes.indexOf(sidePane); + if (currentPosition < (visiblePanes.size() - 1)) { + int newPosition = currentPosition + 1; + visiblePanes.remove(currentPosition); + visiblePanes.add(newPosition, sidePane); + updatePreferredPositions(); + return true; + } + } + return false; } - public void hide(SidePaneType sidePaneType) { + /** + * @return True if sidePane is not already shown which means the view needs to be updated + */ + public boolean show(SidePaneType sidePane) { + if (!visiblePanes.contains(sidePane)) { + visiblePanes.add(sidePane); + preferencesService.getSidePanePreferences().visiblePanes().add(sidePane); + visiblePanes.sorted(new PreferredIndexSort(preferencesService)); + return true; + } + return false; + } + + /** + * @return True if sidePane is visible which means the view needs to be updated + */ + public boolean hide(SidePaneType sidePane) { + if (visiblePanes.contains(sidePane)) { + // TODO('Before closing') + visiblePanes.remove(sidePane); + preferencesService.getSidePanePreferences().visiblePanes().remove(sidePane); + return true; + } else { + return false; + } + } + + /** + * Helper class for sorting visible side panes based on their preferred position. + */ + private static class PreferredIndexSort implements Comparator { + + private final Map preferredPositions; + + public PreferredIndexSort(PreferencesService preferencesService) { + preferredPositions = preferencesService.getSidePanePreferences().getPreferredPositions(); + } + @Override + public int compare(SidePaneType type1, SidePaneType type2) { + int pos1 = preferredPositions.getOrDefault(type1, 0); + int pos2 = preferredPositions.getOrDefault(type2, 0); + return Integer.compare(pos1, pos2); + } } } diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneView.java b/src/main/java/org/jabref/gui/sidepane/SidePaneView.java index a3912f1c535..8ef89e0851f 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneView.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneView.java @@ -1,7 +1,5 @@ package org.jabref.gui.sidepane; -import java.util.Optional; - import javafx.scene.Node; import javafx.scene.layout.BorderPane; import javafx.scene.layout.Priority; diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java index 2382073b37f..32e73fa6990 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java @@ -1,4 +1,7 @@ package org.jabref.gui.sidepane; + + public class SidePaneViewModel { + } From 41dba616af2758b3958674e1877fd04ce00cb95c Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sun, 31 Oct 2021 18:46:35 +0100 Subject: [PATCH 07/36] Bind side pane check menu selection state to side pane visibility --- src/main/java/org/jabref/gui/JabRefFrame.java | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index ec94b8e45c2..286d0372448 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -25,6 +25,7 @@ import javafx.scene.control.Button; import javafx.scene.control.ButtonBar; import javafx.scene.control.ButtonType; +import javafx.scene.control.CheckMenuItem; import javafx.scene.control.ContextMenu; import javafx.scene.control.Menu; import javafx.scene.control.MenuBar; @@ -113,6 +114,7 @@ import org.jabref.gui.shared.PullChangesFromSharedAction; import org.jabref.gui.sidepane.SidePane; import org.jabref.gui.sidepane.SidePaneComponent; +import org.jabref.gui.sidepane.SidePaneContainerView; import org.jabref.gui.sidepane.SidePaneType; import org.jabref.gui.slr.ExistingStudySearchAction; import org.jabref.gui.slr.StartNewStudyAction; @@ -145,6 +147,7 @@ import com.google.common.eventbus.Subscribe; import com.tobiasdiez.easybind.EasyBind; import com.tobiasdiez.easybind.EasyObservableList; +import de.saxsys.mvvmfx.utils.commands.Command; import org.controlsfx.control.PopOver; import org.controlsfx.control.TaskProgressView; import org.fxmisc.richtext.CodeArea; @@ -343,7 +346,7 @@ public void about() { * FIXME: Currently some threads remain and therefore hinder JabRef to be closed properly * * @param filenames the filenames of all currently opened files - used for storing them if prefs openLastEdited is - * set to true + * set to true */ private void tearDownJabRef(List filenames) { if (prefs.getGuiPreferences().shouldOpenLastEdited()) { @@ -941,6 +944,12 @@ private Button createNewEntryFromIdButton() { return newEntryFromIdButton; } + private CheckMenuItem createSidePaneCheckMenuItem(SidePaneContainerView container, ActionFactory factory, SidePaneType paneType, Command toggleCommand) { + CheckMenuItem checkMenuItem = factory.createCheckMenuItem(paneType.getToggleAction(), toggleCommand, (container.isPaneVisibleProperty(paneType).get())); + checkMenuItem.selectedProperty().bindBidirectional(container.isPaneVisibleProperty(paneType)); + return checkMenuItem; + } + private Group createTaskIndicator() { ProgressIndicator indicator = new ProgressIndicator(); indicator.getStyleClass().add("progress-indicatorToolbar"); @@ -1108,7 +1117,7 @@ private boolean readyForAutosave(BibDatabaseContext context) { /** * Opens the import inspection dialog to let the user decide which of the given entries to import. * - * @param panel The BasePanel to add to. + * @param panel The BasePanel to add to. * @param parserResult The entries to add. */ private void addImportedEntries(final LibraryTab panel, final ParserResult parserResult) { @@ -1241,7 +1250,7 @@ public CloseDatabaseAction(LibraryTab libraryTab) { /** * Using this constructor will result in executing the command on the currently open library tab - * */ + */ public CloseDatabaseAction() { this(null); } From c8399507a7c853232b218604c685fdc1e554406f Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sun, 31 Oct 2021 18:51:55 +0100 Subject: [PATCH 08/36] Fix checkstyle --- src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java index 32e73fa6990..4066317fee4 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java @@ -1,7 +1,5 @@ package org.jabref.gui.sidepane; - - public class SidePaneViewModel { } From 69c90df0d42c6ec5a421c3ffea7040fd33da40f4 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sun, 31 Oct 2021 22:38:50 +0100 Subject: [PATCH 09/36] Remove SidePaneViewModel.java --- src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java deleted file mode 100644 index 4066317fee4..00000000000 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java +++ /dev/null @@ -1,5 +0,0 @@ -package org.jabref.gui.sidepane; - -public class SidePaneViewModel { - -} From a9e0419beee0ab9425099ff5a4a5b4426e4a44be Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sun, 31 Oct 2021 22:59:37 +0100 Subject: [PATCH 10/36] Fix view menu for sidebar option out of sync with sidebar state - Fix #8115 - Deprecate the old implementation of Sidepane and remove all of its usage in the rest of the application - Implement the toggle method in SidePaneContainerView --- src/main/java/org/jabref/gui/JabRefFrame.java | 37 +++++------- .../org/jabref/gui/groups/GroupSidePane.java | 3 + .../gui/importer/fetcher/WebSearchPane.java | 1 + .../gui/openoffice/OpenOfficeSidePanel.java | 1 + .../sidepane/GroupsSidePaneHeaderView.java | 5 +- .../org/jabref/gui/sidepane/SidePane.java | 3 + .../gui/sidepane/SidePaneComponent.java | 19 +------ .../gui/sidepane/SidePaneContainerView.java | 56 +++++++++++++------ .../sidepane/SidePaneContainerViewModel.java | 43 +++++++++++++- .../gui/sidepane/SidePaneHeaderView.java | 11 ++-- 10 files changed, 110 insertions(+), 69 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index 286d0372448..036cae4cf4f 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -112,8 +112,6 @@ import org.jabref.gui.search.RebuildFulltextSearchIndexAction; import org.jabref.gui.shared.ConnectToSharedDatabaseCommand; import org.jabref.gui.shared.PullChangesFromSharedAction; -import org.jabref.gui.sidepane.SidePane; -import org.jabref.gui.sidepane.SidePaneComponent; import org.jabref.gui.sidepane.SidePaneContainerView; import org.jabref.gui.sidepane.SidePaneType; import org.jabref.gui.slr.ExistingStudySearchAction; @@ -147,7 +145,6 @@ import com.google.common.eventbus.Subscribe; import com.tobiasdiez.easybind.EasyBind; import com.tobiasdiez.easybind.EasyObservableList; -import de.saxsys.mvvmfx.utils.commands.Command; import org.controlsfx.control.PopOver; import org.controlsfx.control.TaskProgressView; import org.fxmisc.richtext.CodeArea; @@ -176,7 +173,7 @@ public class JabRefFrame extends BorderPane { private final CountingUndoManager undoManager; private final PushToApplicationsManager pushToApplicationsManager; private final DialogService dialogService; - private SidePane sidePane; + private SidePaneContainerView sidePaneContainerView; private TabPane tabbedPane; private PopOver progressViewPopOver; private PopOver entryFromIdPopOver; @@ -432,8 +429,8 @@ private void initLayout() { head.setSpacing(0d); setTop(head); - splitPane.getItems().addAll(sidePane, tabbedPane); - SplitPane.setResizableWithParent(sidePane, false); + splitPane.getItems().addAll(sidePaneContainerView, tabbedPane); + SplitPane.setResizableWithParent(sidePaneContainerView, false); // We need to wait with setting the divider since it gets reset a few times during the initial set-up mainStage.showingProperty().addListener(new ChangeListener<>() { @@ -442,15 +439,14 @@ private void initLayout() { public void changed(ObservableValue observable, Boolean oldValue, Boolean showing) { if (showing) { setDividerPosition(); - - EasyBind.subscribe(sidePane.visibleProperty(), visible -> { + EasyBind.subscribe(sidePaneContainerView.visibleProperty(), visible -> { if (visible) { - if (!splitPane.getItems().contains(sidePane)) { - splitPane.getItems().add(0, sidePane); + if (!splitPane.getItems().contains(sidePaneContainerView)) { + splitPane.getItems().add(0, sidePaneContainerView); setDividerPosition(); } } else { - splitPane.getItems().remove(sidePane); + splitPane.getItems().remove(sidePaneContainerView); } }); @@ -575,8 +571,7 @@ public void showLibraryTab(LibraryTab libraryTab) { } public void init() { - sidePane = new SidePane(prefs, taskExecutor, dialogService, stateManager, undoManager); - + sidePaneContainerView = new SidePaneContainerView(prefs, taskExecutor, dialogService, stateManager, undoManager); tabbedPane = new TabPane(); tabbedPane.setTabDragPolicy(TabPane.TabDragPolicy.REORDER); @@ -840,14 +835,10 @@ private MenuBar createMenu() { factory.createMenuItem(StandardActions.REBUILD_FULLTEXT_SEARCH_INDEX, new RebuildFulltextSearchIndexAction(stateManager, this::getCurrentLibraryTab, dialogService, prefs.getFilePreferences())) ); - SidePaneComponent webSearch = sidePane.getComponent(SidePaneType.WEB_SEARCH); - SidePaneComponent groups = sidePane.getComponent(SidePaneType.GROUPS); - SidePaneComponent openOffice = sidePane.getComponent(SidePaneType.OPEN_OFFICE); - view.getItems().addAll( - factory.createCheckMenuItem(webSearch.getToggleAction(), webSearch.getToggleCommand(), sidePane.isComponentVisible(SidePaneType.WEB_SEARCH)), - factory.createCheckMenuItem(groups.getToggleAction(), groups.getToggleCommand(), sidePane.isComponentVisible(SidePaneType.GROUPS)), - factory.createCheckMenuItem(openOffice.getToggleAction(), openOffice.getToggleCommand(), sidePane.isComponentVisible(SidePaneType.OPEN_OFFICE)), + createSidePaneCheckMenuItem(sidePaneContainerView, factory, SidePaneType.WEB_SEARCH), + createSidePaneCheckMenuItem(sidePaneContainerView, factory, SidePaneType.GROUPS), + createSidePaneCheckMenuItem(sidePaneContainerView, factory, SidePaneType.OPEN_OFFICE), new SeparatorMenuItem(), @@ -944,9 +935,9 @@ private Button createNewEntryFromIdButton() { return newEntryFromIdButton; } - private CheckMenuItem createSidePaneCheckMenuItem(SidePaneContainerView container, ActionFactory factory, SidePaneType paneType, Command toggleCommand) { - CheckMenuItem checkMenuItem = factory.createCheckMenuItem(paneType.getToggleAction(), toggleCommand, (container.isPaneVisibleProperty(paneType).get())); - checkMenuItem.selectedProperty().bindBidirectional(container.isPaneVisibleProperty(paneType)); + private CheckMenuItem createSidePaneCheckMenuItem(SidePaneContainerView container, ActionFactory factory, SidePaneType sidePane) { + CheckMenuItem checkMenuItem = factory.createCheckMenuItem(sidePane.getToggleAction(), container.getToggleCommandFor(sidePane), container.sidePaneVisibleProperty(sidePane).get()); + EasyBind.subscribe(container.sidePaneVisibleProperty(sidePane), checkMenuItem::setSelected); return checkMenuItem; } diff --git a/src/main/java/org/jabref/gui/groups/GroupSidePane.java b/src/main/java/org/jabref/gui/groups/GroupSidePane.java index 86bc9cfb725..6a48f5404fa 100644 --- a/src/main/java/org/jabref/gui/groups/GroupSidePane.java +++ b/src/main/java/org/jabref/gui/groups/GroupSidePane.java @@ -21,7 +21,10 @@ /** * The groups side pane. + * @deprecated */ + +@Deprecated public class GroupSidePane extends SidePaneComponent { private final PreferencesService preferences; diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java index ad7b69cd21c..c0e57fa1d44 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java @@ -31,6 +31,7 @@ import com.tobiasdiez.easybind.EasyBind; +@Deprecated public class WebSearchPane extends SidePaneComponent { private static final PseudoClass QUERY_INVALID = PseudoClass.getPseudoClass("invalid"); diff --git a/src/main/java/org/jabref/gui/openoffice/OpenOfficeSidePanel.java b/src/main/java/org/jabref/gui/openoffice/OpenOfficeSidePanel.java index 569b5b78428..1503d90a60e 100644 --- a/src/main/java/org/jabref/gui/openoffice/OpenOfficeSidePanel.java +++ b/src/main/java/org/jabref/gui/openoffice/OpenOfficeSidePanel.java @@ -17,6 +17,7 @@ import org.jabref.logic.openoffice.OpenOfficePreferences; import org.jabref.preferences.PreferencesService; +@Deprecated public class OpenOfficeSidePanel extends SidePaneComponent { private final TaskExecutor taskExecutor; diff --git a/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneHeaderView.java b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneHeaderView.java index 112f6d4306b..064a5bbf9c6 100644 --- a/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneHeaderView.java +++ b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneHeaderView.java @@ -3,8 +3,6 @@ import javafx.scene.control.Button; import org.jabref.gui.DialogService; -import org.jabref.gui.Globals; -import org.jabref.gui.actions.ActionFactory; import org.jabref.gui.actions.SimpleCommand; import org.jabref.gui.groups.GroupModeViewModel; import org.jabref.gui.groups.GroupViewMode; @@ -26,8 +24,7 @@ public GroupsSidePaneHeaderView(SidePaneType sidePaneType, SimpleCommand closeCo private void setupIntersectionUnionToggle() { addButtonAtPosition(intersectionUnionToggle, 2); - ActionFactory factory = new ActionFactory(Globals.getKeyPrefs()); - factory.configureIconButton(getSidePaneType().getToggleAction(), new ToggleUnionIntersectionAction(), intersectionUnionToggle); + intersectionUnionToggle.setOnAction(event -> new ToggleUnionIntersectionAction().execute()); } public void afterOpening() { diff --git a/src/main/java/org/jabref/gui/sidepane/SidePane.java b/src/main/java/org/jabref/gui/sidepane/SidePane.java index ef565fbbf2d..3c109df7a03 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePane.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePane.java @@ -23,7 +23,10 @@ /** * Manages which {@link SidePaneComponent}s are shown. + * + * @deprecated */ +@Deprecated public class SidePane extends VBox { private final Map components = new LinkedHashMap<>(); diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java b/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java index 6a039ebb081..fc87cbdc644 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java @@ -3,8 +3,6 @@ import java.util.Collections; import java.util.List; -import javafx.beans.property.BooleanProperty; -import javafx.beans.property.SimpleBooleanProperty; import javafx.scene.Node; import javafx.scene.control.Button; import javafx.scene.control.Label; @@ -19,6 +17,7 @@ import org.jabref.gui.icon.JabRefIcon; import org.jabref.logic.l10n.Localization; +@Deprecated public abstract class SidePaneComponent { private final SidePane sidePane; @@ -26,7 +25,6 @@ public abstract class SidePaneComponent { private final JabRefIcon icon; private final String title; private Node contentNode; - private final BooleanProperty visible = new SimpleBooleanProperty(); public SidePaneComponent(SidePane sidePane, JabRefIcon icon, String title) { this.sidePane = sidePane; @@ -35,25 +33,11 @@ public SidePaneComponent(SidePane sidePane, JabRefIcon icon, String title) { this.toggleCommand = new ToggleCommand(this); } - public BooleanProperty visibleProperty() { - return visible; - } - - public boolean isVisible() { - return visibleProperty().get(); - } - - // TODO ('hide() is only used in SidePane') protected void hide() { - visible.setValue(false); - // TODO('Model should not update the view directly') sidePane.hide(this.getType()); } - // TODO ('show() is only used in SidePane') protected void show() { - visible.setValue(true); - // TODO('Model should not update the view directly') sidePane.show(this.getType()); } @@ -113,7 +97,6 @@ public final Node getContentPane() { public final Node getHeader() { Button close = IconTheme.JabRefIcons.CLOSE.asButton(); close.setTooltip(new Tooltip(Localization.lang("Hide panel"))); - // TODO (Use visibleProperty inside onAction) close.setOnAction(event -> hide()); Button up = IconTheme.JabRefIcons.UP.asButton(); diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java index b8d366c83a8..c7c60cd275a 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java @@ -1,14 +1,11 @@ package org.jabref.gui.sidepane; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; -import java.util.Set; import javax.swing.undo.UndoManager; import javafx.beans.property.BooleanProperty; -import javafx.beans.property.SimpleBooleanProperty; import javafx.scene.Node; import javafx.scene.layout.Priority; import javafx.scene.layout.VBox; @@ -37,11 +34,6 @@ public class SidePaneContainerView extends VBox { private final StateManager stateManager; private final UndoManager undoManager; - // Probably should go into the view model - private final BooleanProperty groupsPaneVisible = new SimpleBooleanProperty(); - private final BooleanProperty openOfficePaneVisible = new SimpleBooleanProperty(); - private final BooleanProperty webSearchPaneVisible = new SimpleBooleanProperty(); - public SidePaneContainerView(PreferencesService preferencesService, TaskExecutor taskExecutor, DialogService dialogService, @@ -96,9 +88,9 @@ private SidePaneView createSidePaneView(SidePaneType sidePaneType, Node paneCont return new SidePaneView(paneHeaderView, paneContent, Priority.NEVER); } - private void showSidePanes(Set toShowSidePanes) { + private void showVisibleSidePanes() { getChildren().clear(); - toShowSidePanes.forEach(type -> { + viewModel.getVisiblePanes().forEach(type -> { SidePaneView view = getSidePaneView(type); getChildren().add(view); VBox.setVgrow(view, view.getResizePolicy()); @@ -108,6 +100,9 @@ private void showSidePanes(Set toShowSidePanes) { private void show(SidePaneType sidePane) { if (viewModel.show(sidePane)) { updateView(); + if (sidePane == GROUPS) { + ((GroupsSidePaneView) getSidePaneView(sidePane)).afterOpening(); + } } } @@ -129,19 +124,34 @@ private void moveDown(SidePaneType sidePane) { } } + /** + * If the given component is visible it will be hidden and the other way around. + */ + private void toggle(SidePaneType sidePane) { + if (viewModel.isSidePaneVisible(sidePane)) { + hide(sidePane); + } else { + show(sidePane); + } + } + private void updateView() { - showSidePanes(new HashSet<>(viewModel.getVisiblePanes())); + showVisibleSidePanes(); setVisible(!viewModel.getVisiblePanes().isEmpty()); } - public BooleanProperty isPaneVisibleProperty(SidePaneType sidePaneType) { - return switch (sidePaneType) { - case GROUPS -> groupsPaneVisible; - case WEB_SEARCH -> webSearchPaneVisible; - case OPEN_OFFICE -> openOfficePaneVisible; + public BooleanProperty sidePaneVisibleProperty(SidePaneType sidePane) { + return switch (sidePane) { + case GROUPS -> viewModel.groupsSidePaneVisibleProperty(); + case WEB_SEARCH -> viewModel.webSearchSidePaneVisibleProperty(); + case OPEN_OFFICE -> viewModel.openOfficePaneVisibleProperty(); }; } + public ToggleCommand getToggleCommandFor(SidePaneType sidePane) { + return new ToggleCommand(sidePane); + } + private class CloseSidePaneAction extends SimpleCommand { private final SidePaneType toCloseSidePane; @@ -180,4 +190,18 @@ public void execute() { moveDown(toMoveDownSidePane); } } + + public class ToggleCommand extends SimpleCommand { + + private final SidePaneType sidePane; + + public ToggleCommand(SidePaneType sidePane) { + this.sidePane = sidePane; + } + + @Override + public void execute() { + toggle(sidePane); + } + } } diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java index f1e8dbb7086..ce93a2e7cec 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java @@ -5,7 +5,10 @@ import java.util.Map; import java.util.stream.IntStream; +import javafx.beans.property.BooleanProperty; +import javafx.beans.property.SimpleBooleanProperty; import javafx.collections.FXCollections; +import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; import org.jabref.gui.AbstractViewModel; @@ -16,8 +19,31 @@ public class SidePaneContainerViewModel extends AbstractViewModel { // TODO('Use preferencesService.getSidePanePreferences().visiblePanes() as the single source of truth') private final ObservableList visiblePanes = FXCollections.observableArrayList(); + private final BooleanProperty groupsPaneVisible = new SimpleBooleanProperty(); + private final BooleanProperty openOfficePaneVisible = new SimpleBooleanProperty(); + private final BooleanProperty webSearchPaneVisible = new SimpleBooleanProperty(); + public SidePaneContainerViewModel(PreferencesService preferencesService) { this.preferencesService = preferencesService; + visiblePanes.addListener((ListChangeListener) change -> { + while (change.next()) { + if (change.wasAdded()) { + SidePaneType addedSidePane = change.getAddedSubList().get(0); + switch (addedSidePane) { + case GROUPS -> groupsPaneVisible.setValue(true); + case OPEN_OFFICE -> openOfficePaneVisible.setValue(true); + case WEB_SEARCH -> webSearchPaneVisible.setValue(true); + } + } else if (change.wasRemoved()) { + SidePaneType removedSidePane = change.getRemoved().get(0); + switch (removedSidePane) { + case GROUPS -> groupsPaneVisible.setValue(false); + case OPEN_OFFICE -> openOfficePaneVisible.setValue(false); + case WEB_SEARCH -> webSearchPaneVisible.setValue(false); + } + } + } + }); } /** @@ -88,7 +114,6 @@ public boolean show(SidePaneType sidePane) { */ public boolean hide(SidePaneType sidePane) { if (visiblePanes.contains(sidePane)) { - // TODO('Before closing') visiblePanes.remove(sidePane); preferencesService.getSidePanePreferences().visiblePanes().remove(sidePane); return true; @@ -97,6 +122,22 @@ public boolean hide(SidePaneType sidePane) { } } + public BooleanProperty groupsSidePaneVisibleProperty() { + return groupsPaneVisible; + } + + public BooleanProperty openOfficePaneVisibleProperty() { + return openOfficePaneVisible; + } + + public BooleanProperty webSearchSidePaneVisibleProperty() { + return webSearchPaneVisible; + } + + public boolean isSidePaneVisible(SidePaneType sidePane) { + return visiblePanes.contains(sidePane); + } + /** * Helper class for sorting visible side panes based on their preferred position. */ diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneHeaderView.java b/src/main/java/org/jabref/gui/sidepane/SidePaneHeaderView.java index 3271c9fd212..69a620ad45a 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneHeaderView.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneHeaderView.java @@ -6,8 +6,6 @@ import javafx.scene.layout.BorderPane; import javafx.scene.layout.HBox; -import org.jabref.gui.Globals; -import org.jabref.gui.actions.ActionFactory; import org.jabref.gui.actions.SimpleCommand; import org.jabref.gui.icon.IconTheme; import org.jabref.logic.l10n.Localization; @@ -29,18 +27,17 @@ public SidePaneHeaderView(SidePaneType sidePaneType, SimpleCommand closeCommand, } private void initView() { - ActionFactory factory = new ActionFactory(Globals.getKeyPrefs()); Button closeButton = IconTheme.JabRefIcons.CLOSE.asButton(); closeButton.setTooltip(new Tooltip(Localization.lang("Hide panel"))); - factory.configureIconButton(sidePaneType.getToggleAction(), closeCommand, closeButton); + closeButton.setOnAction(e -> closeCommand.execute()); Button upButton = IconTheme.JabRefIcons.UP.asButton(); upButton.setTooltip(new Tooltip(Localization.lang("Move panel up"))); - factory.configureIconButton(sidePaneType.getToggleAction(), moveUpCommand, upButton); + upButton.setOnAction(e -> moveUpCommand.execute()); Button downButton = IconTheme.JabRefIcons.DOWN.asButton(); downButton.setTooltip(new Tooltip(Localization.lang("Move panel down"))); - factory.configureIconButton(sidePaneType.getToggleAction(), moveDownCommand, downButton); + downButton.setOnAction(e -> moveDownCommand.execute()); this.buttonContainer = new HBox(); buttonContainer.getChildren().addAll(upButton, downButton, closeButton); @@ -52,7 +49,7 @@ private void initView() { } protected void addButtonAtPosition(Button button, int position) { - this.buttonContainer.getChildren().set(position, button); + this.buttonContainer.getChildren().add(position, button); } public SidePaneType getSidePaneType() { From 289165bad1b79e4864b52d19d5d0caaab098b99c Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sun, 31 Oct 2021 23:14:48 +0100 Subject: [PATCH 11/36] Remove unused constructor parameters --- .../java/org/jabref/gui/sidepane/GroupsSidePaneView.java | 5 +---- .../java/org/jabref/gui/sidepane/SidePaneContainerView.java | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneView.java b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneView.java index e80fc9e1add..e5d92a9c189 100644 --- a/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneView.java +++ b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneView.java @@ -3,11 +3,8 @@ import javafx.scene.Node; import javafx.scene.layout.Priority; -import org.jabref.gui.DialogService; -import org.jabref.preferences.PreferencesService; - public class GroupsSidePaneView extends SidePaneView { - public GroupsSidePaneView(GroupsSidePaneHeaderView sidePaneHeaderView, Node content, Priority resizePolicy, PreferencesService preferences, DialogService dialogService) { + public GroupsSidePaneView(GroupsSidePaneHeaderView sidePaneHeaderView, Node content, Priority resizePolicy) { super(sidePaneHeaderView, content, resizePolicy); } diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java index c7c60cd275a..0e45c0b6154 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java @@ -68,7 +68,7 @@ private SidePaneView createGroupsSidePaneView() { new CloseSidePaneAction(GROUPS), new MoveUpAction(GROUPS), new MoveDownAction(GROUPS), preferencesService, dialogService); GroupTreeView groupsContent = new GroupTreeView(taskExecutor, stateManager, preferencesService, dialogService); - return new GroupsSidePaneView(groupsHeader, groupsContent, Priority.ALWAYS, preferencesService, dialogService); + return new GroupsSidePaneView(groupsHeader, groupsContent, Priority.ALWAYS); } private SidePaneView createOpenOfficeSidePaneView() { From d807d9be0a32d49c209ffab90b6ccde33b514848 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sun, 31 Oct 2021 23:36:56 +0100 Subject: [PATCH 12/36] Remove deprecated Sidepane component --- .../org/jabref/gui/groups/GroupSidePane.java | 100 --------- .../gui/importer/fetcher/WebSearchPane.java | 132 ----------- .../gui/openoffice/OpenOfficeSidePanel.java | 74 ------- .../org/jabref/gui/sidepane/SidePane.java | 208 ------------------ .../gui/sidepane/SidePaneComponent.java | 158 ------------- 5 files changed, 672 deletions(-) delete mode 100644 src/main/java/org/jabref/gui/groups/GroupSidePane.java delete mode 100644 src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java delete mode 100644 src/main/java/org/jabref/gui/openoffice/OpenOfficeSidePanel.java delete mode 100644 src/main/java/org/jabref/gui/sidepane/SidePane.java delete mode 100644 src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java diff --git a/src/main/java/org/jabref/gui/groups/GroupSidePane.java b/src/main/java/org/jabref/gui/groups/GroupSidePane.java deleted file mode 100644 index 6a48f5404fa..00000000000 --- a/src/main/java/org/jabref/gui/groups/GroupSidePane.java +++ /dev/null @@ -1,100 +0,0 @@ -package org.jabref.gui.groups; - -import java.util.Collections; -import java.util.List; - -import javafx.scene.Node; -import javafx.scene.control.Button; -import javafx.scene.layout.Priority; - -import org.jabref.gui.DialogService; -import org.jabref.gui.StateManager; -import org.jabref.gui.actions.Action; -import org.jabref.gui.actions.StandardActions; -import org.jabref.gui.icon.IconTheme; -import org.jabref.gui.sidepane.SidePane; -import org.jabref.gui.sidepane.SidePaneComponent; -import org.jabref.gui.sidepane.SidePaneType; -import org.jabref.gui.util.TaskExecutor; -import org.jabref.logic.l10n.Localization; -import org.jabref.preferences.PreferencesService; - -/** - * The groups side pane. - * @deprecated - */ - -@Deprecated -public class GroupSidePane extends SidePaneComponent { - - private final PreferencesService preferences; - private final DialogService dialogService; - private final TaskExecutor taskExecutor; - private final StateManager stateManager; - private final Button intersectionUnionToggle = IconTheme.JabRefIcons.GROUP_INTERSECTION.asButton(); - - public GroupSidePane(SidePane sidePane, TaskExecutor taskExecutor, StateManager stateManager, PreferencesService preferences, DialogService dialogService) { - super(sidePane, IconTheme.JabRefIcons.TOGGLE_GROUPS, Localization.lang("Groups")); - this.preferences = preferences; - this.taskExecutor = taskExecutor; - this.stateManager = stateManager; - this.dialogService = dialogService; - } - - @Override - protected List getAdditionalHeaderButtons() { - intersectionUnionToggle.setOnAction(event -> toggleUnionIntersection()); - return Collections.singletonList(intersectionUnionToggle); - } - - @Override - public Priority getResizePolicy() { - return Priority.ALWAYS; - } - - @Override - public void beforeClosing() { - preferences.getSidePanePreferences().visiblePanes().remove(SidePaneType.GROUPS); - } - - @Override - public void afterOpening() { - preferences.getSidePanePreferences().visiblePanes().add(SidePaneType.GROUPS); - setGraphicsAndTooltipForButton(preferences.getGroupViewMode()); - } - - @Override - public Action getToggleAction() { - return StandardActions.TOGGLE_GROUPS; - } - - private void toggleUnionIntersection() { - GroupViewMode mode = preferences.getGroupViewMode(); - - if (mode == GroupViewMode.UNION) { - preferences.setGroupViewMode(GroupViewMode.INTERSECTION); - dialogService.notify(Localization.lang("Group view mode set to intersection")); - } else if (mode == GroupViewMode.INTERSECTION) { - preferences.setGroupViewMode(GroupViewMode.UNION); - dialogService.notify(Localization.lang("Group view mode set to union")); - } - - setGraphicsAndTooltipForButton(mode); - } - - private void setGraphicsAndTooltipForButton(GroupViewMode mode) { - GroupModeViewModel modeViewModel = new GroupModeViewModel(mode); - intersectionUnionToggle.setGraphic(modeViewModel.getUnionIntersectionGraphic()); - intersectionUnionToggle.setTooltip(modeViewModel.getUnionIntersectionTooltip()); - } - - @Override - protected Node createContentPane() { - return new GroupTreeView(taskExecutor, stateManager, preferences, dialogService); - } - - @Override - public SidePaneType getType() { - return SidePaneType.GROUPS; - } -} diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java deleted file mode 100644 index c0e57fa1d44..00000000000 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java +++ /dev/null @@ -1,132 +0,0 @@ -package org.jabref.gui.importer.fetcher; - -import javafx.css.PseudoClass; -import javafx.geometry.Pos; -import javafx.scene.Node; -import javafx.scene.control.Button; -import javafx.scene.control.ComboBox; -import javafx.scene.control.TextField; -import javafx.scene.control.Tooltip; -import javafx.scene.input.KeyCode; -import javafx.scene.layout.HBox; -import javafx.scene.layout.Priority; -import javafx.scene.layout.StackPane; -import javafx.scene.layout.VBox; - -import org.jabref.gui.DialogService; -import org.jabref.gui.StateManager; -import org.jabref.gui.actions.Action; -import org.jabref.gui.actions.ActionFactory; -import org.jabref.gui.actions.StandardActions; -import org.jabref.gui.help.HelpAction; -import org.jabref.gui.icon.IconTheme; -import org.jabref.gui.search.SearchTextField; -import org.jabref.gui.sidepane.SidePane; -import org.jabref.gui.sidepane.SidePaneComponent; -import org.jabref.gui.sidepane.SidePaneType; -import org.jabref.gui.util.ViewModelListCellFactory; -import org.jabref.logic.importer.SearchBasedFetcher; -import org.jabref.logic.l10n.Localization; -import org.jabref.preferences.PreferencesService; - -import com.tobiasdiez.easybind.EasyBind; - -@Deprecated -public class WebSearchPane extends SidePaneComponent { - - private static final PseudoClass QUERY_INVALID = PseudoClass.getPseudoClass("invalid"); - - private final PreferencesService preferences; - private final WebSearchPaneViewModel viewModel; - - public WebSearchPane(SidePane sidePane, PreferencesService preferences, DialogService dialogService, StateManager stateManager) { - super(sidePane, IconTheme.JabRefIcons.WWW, Localization.lang("Web search")); - this.preferences = preferences; - this.viewModel = new WebSearchPaneViewModel(preferences, dialogService, stateManager); - } - - @Override - public Action getToggleAction() { - return StandardActions.TOGGLE_WEB_SEARCH; - } - - @Override - protected Node createContentPane() { - // Setup combo box for fetchers - ComboBox fetchers = new ComboBox<>(); - new ViewModelListCellFactory() - .withText(SearchBasedFetcher::getName) - .install(fetchers); - fetchers.itemsProperty().bind(viewModel.fetchersProperty()); - fetchers.valueProperty().bindBidirectional(viewModel.selectedFetcherProperty()); - fetchers.setMaxWidth(Double.POSITIVE_INFINITY); - - // Create help button for currently selected fetcher - StackPane helpButtonContainer = new StackPane(); - ActionFactory factory = new ActionFactory(preferences.getKeyBindingRepository()); - EasyBind.subscribe(viewModel.selectedFetcherProperty(), fetcher -> { - if ((fetcher != null) && fetcher.getHelpPage().isPresent()) { - Button helpButton = factory.createIconButton(StandardActions.HELP, new HelpAction(fetcher.getHelpPage().get())); - helpButtonContainer.getChildren().setAll(helpButton); - } else { - helpButtonContainer.getChildren().clear(); - } - }); - HBox fetcherContainer = new HBox(fetchers, helpButtonContainer); - HBox.setHgrow(fetchers, Priority.ALWAYS); - - // Create text field for query input - TextField query = SearchTextField.create(); - query.getStyleClass().add("searchBar"); - - viewModel.queryProperty().bind(query.textProperty()); - EasyBind.subscribe(viewModel.queryValidationStatus().validProperty(), - valid -> { - if (!valid && viewModel.queryValidationStatus().getHighestMessage().isPresent()) { - query.setTooltip(new Tooltip(viewModel.queryValidationStatus().getHighestMessage().get().getMessage())); - query.pseudoClassStateChanged(QUERY_INVALID, true); - } else { - query.setTooltip(null); - query.pseudoClassStateChanged(QUERY_INVALID, false); - } - }); - - // Allows to trigger search on pressing enter - query.setOnKeyPressed(event -> { - if (event.getCode() == KeyCode.ENTER) { - viewModel.search(); - } - }); - - // Create button that triggers search - Button search = new Button(Localization.lang("Search")); - search.setDefaultButton(false); - search.setOnAction(event -> viewModel.search()); - - // Put everything together - VBox container = new VBox(); - container.setAlignment(Pos.CENTER); - container.getChildren().addAll(fetcherContainer, query, search); - return container; - } - - @Override - public SidePaneType getType() { - return SidePaneType.WEB_SEARCH; - } - - @Override - public void beforeClosing() { - preferences.getSidePanePreferences().visiblePanes().remove(SidePaneType.WEB_SEARCH); - } - - @Override - public void afterOpening() { - preferences.getSidePanePreferences().visiblePanes().add(SidePaneType.WEB_SEARCH); - } - - @Override - public Priority getResizePolicy() { - return Priority.NEVER; - } -} diff --git a/src/main/java/org/jabref/gui/openoffice/OpenOfficeSidePanel.java b/src/main/java/org/jabref/gui/openoffice/OpenOfficeSidePanel.java deleted file mode 100644 index 1503d90a60e..00000000000 --- a/src/main/java/org/jabref/gui/openoffice/OpenOfficeSidePanel.java +++ /dev/null @@ -1,74 +0,0 @@ -package org.jabref.gui.openoffice; - -import javax.swing.undo.UndoManager; - -import javafx.scene.Node; -import javafx.scene.layout.Priority; - -import org.jabref.gui.DialogService; -import org.jabref.gui.StateManager; -import org.jabref.gui.actions.Action; -import org.jabref.gui.actions.StandardActions; -import org.jabref.gui.icon.IconTheme; -import org.jabref.gui.sidepane.SidePane; -import org.jabref.gui.sidepane.SidePaneComponent; -import org.jabref.gui.sidepane.SidePaneType; -import org.jabref.gui.util.TaskExecutor; -import org.jabref.logic.openoffice.OpenOfficePreferences; -import org.jabref.preferences.PreferencesService; - -@Deprecated -public class OpenOfficeSidePanel extends SidePaneComponent { - - private final TaskExecutor taskExecutor; - private final PreferencesService preferencesService; - private final DialogService dialogService; - private final StateManager stateManager; - private final UndoManager undoManager; - private final OpenOfficePreferences ooPrefs; - - public OpenOfficeSidePanel(SidePane sidePane, - TaskExecutor taskExecutor, - PreferencesService preferencesService, - DialogService dialogService, - StateManager stateManager, - UndoManager undoManager) { - super(sidePane, IconTheme.JabRefIcons.FILE_OPENOFFICE, "OpenOffice/LibreOffice"); - this.taskExecutor = taskExecutor; - this.preferencesService = preferencesService; - this.dialogService = dialogService; - this.stateManager = stateManager; - this.undoManager = undoManager; - this.ooPrefs = preferencesService.getOpenOfficePreferences(); - } - - @Override - public void beforeClosing() { - preferencesService.getSidePanePreferences().visiblePanes().remove(SidePaneType.OPEN_OFFICE); - } - - @Override - public void afterOpening() { - preferencesService.getSidePanePreferences().visiblePanes().add(SidePaneType.OPEN_OFFICE); - } - - @Override - public Priority getResizePolicy() { - return Priority.NEVER; - } - - @Override - public Action getToggleAction() { - return StandardActions.TOOGLE_OO; - } - - @Override - protected Node createContentPane() { - return new OpenOfficePanel(preferencesService, ooPrefs, preferencesService.getKeyBindingRepository(), taskExecutor, dialogService, stateManager, undoManager).getContent(); - } - - @Override - public SidePaneType getType() { - return SidePaneType.OPEN_OFFICE; - } -} diff --git a/src/main/java/org/jabref/gui/sidepane/SidePane.java b/src/main/java/org/jabref/gui/sidepane/SidePane.java deleted file mode 100644 index 3c109df7a03..00000000000 --- a/src/main/java/org/jabref/gui/sidepane/SidePane.java +++ /dev/null @@ -1,208 +0,0 @@ -package org.jabref.gui.sidepane; - -import java.util.Collection; -import java.util.Comparator; -import java.util.HashMap; -import java.util.LinkedHashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; - -import javax.swing.undo.UndoManager; - -import javafx.scene.layout.BorderPane; -import javafx.scene.layout.VBox; - -import org.jabref.gui.DialogService; -import org.jabref.gui.StateManager; -import org.jabref.gui.groups.GroupSidePane; -import org.jabref.gui.importer.fetcher.WebSearchPane; -import org.jabref.gui.openoffice.OpenOfficeSidePanel; -import org.jabref.gui.util.TaskExecutor; -import org.jabref.preferences.PreferencesService; - -/** - * Manages which {@link SidePaneComponent}s are shown. - * - * @deprecated - */ -@Deprecated -public class SidePane extends VBox { - - private final Map components = new LinkedHashMap<>(); - private final List visibleComponents = new LinkedList<>(); - - private final PreferencesService preferencesService; - private final TaskExecutor taskExecutor; - private final DialogService dialogService; - private final StateManager stateManager; - private final UndoManager undoManager; - - public SidePane(PreferencesService preferencesService, - TaskExecutor taskExecutor, - DialogService dialogService, - StateManager stateManager, - UndoManager undoManager) { - this.preferencesService = preferencesService; - this.taskExecutor = taskExecutor; - this.dialogService = dialogService; - this.stateManager = stateManager; - this.undoManager = undoManager; - - setId("sidePane"); - - preferencesService.getSidePanePreferences().visiblePanes().forEach(this::show); - - updateView(); - } - - public boolean isComponentVisible(SidePaneType type) { - return visibleComponents.contains(getComponent(type)); - } - - public SidePaneComponent getComponent(SidePaneType type) { - SidePaneComponent component = components.get(type); - if (component == null) { - component = switch (type) { - case OPEN_OFFICE -> new OpenOfficeSidePanel(this, taskExecutor, preferencesService, dialogService, stateManager, undoManager); - case WEB_SEARCH -> new WebSearchPane(this, preferencesService, dialogService, stateManager); - case GROUPS -> new GroupSidePane(this, taskExecutor, stateManager, preferencesService, dialogService); - }; - components.put(component.getType(), component); - } - return component; - } - - /** - * If the given component is visible it will be hidden and the other way around. - */ - protected void toggle(SidePaneType type) { - if (isComponentVisible(type)) { - hide(type); - } else { - show(type); - } - } - - /** - * Makes sure that the given component is visible. - */ - protected void show(SidePaneType type) { - SidePaneComponent component = getComponent(type); - if (!visibleComponents.contains(component)) { - // Add the new component - visibleComponents.add(component); - - // Sort the visible components by their preferred position - visibleComponents.sort(new PreferredIndexSort(preferencesService)); - - updateView(); - - component.afterOpening(); - } - } - - /** - * Makes sure that the given component is not visible. - */ - protected void hide(SidePaneType type) { - SidePaneComponent component = getComponent(type); - if (visibleComponents.contains(component)) { - component.beforeClosing(); - - visibleComponents.remove(component); - - updateView(); - } - } - - /** - * Stores the current configuration of visible components in the preferences, - * so that we show components at the preferred position next time. - */ - private void updatePreferredPositions() { - Map preferredPositions = new HashMap<>(preferencesService.getSidePanePreferences().getPreferredPositions()); - - // Use the currently shown positions of all visible components - int index = 0; - for (SidePaneComponent comp : visibleComponents) { - preferredPositions.put(comp.getType(), index); - index++; - } - preferencesService.getSidePanePreferences().setPreferredPositions(preferredPositions); - } - - /** - * Moves the given component up. - */ - protected void moveUp(SidePaneComponent component) { - if (visibleComponents.contains(component)) { - int currentPosition = visibleComponents.indexOf(component); - if (currentPosition > 0) { - int newPosition = currentPosition - 1; - visibleComponents.remove(currentPosition); - visibleComponents.add(newPosition, component); - - updatePreferredPositions(); - updateView(); - } - } - } - - /** - * Moves the given component down. - */ - protected void moveDown(SidePaneComponent comp) { - if (visibleComponents.contains(comp)) { - int currentPosition = visibleComponents.indexOf(comp); - if (currentPosition < (visibleComponents.size() - 1)) { - int newPosition = currentPosition + 1; - visibleComponents.remove(currentPosition); - visibleComponents.add(newPosition, comp); - - updatePreferredPositions(); - updateView(); - } - } - } - - /** - * Updates the view to reflect changes to visible components. - */ - private void updateView() { - setComponents(visibleComponents); - setVisible(!visibleComponents.isEmpty()); - } - - private void setComponents(Collection components) { - getChildren().clear(); - - for (SidePaneComponent component : components) { - BorderPane node = new BorderPane(); - node.getStyleClass().add("sidePaneComponent"); - node.setTop(component.getHeader()); - node.setCenter(component.getContentPane()); - getChildren().add(node); - VBox.setVgrow(node, component.getResizePolicy()); - } - } - - /** - * Helper class for sorting visible components based on their preferred position. - */ - private static class PreferredIndexSort implements Comparator { - - private final Map preferredPositions; - - public PreferredIndexSort(PreferencesService preferencesService) { - preferredPositions = preferencesService.getSidePanePreferences().getPreferredPositions(); - } - - @Override - public int compare(SidePaneComponent comp1, SidePaneComponent comp2) { - int pos1 = preferredPositions.getOrDefault(comp1.getType(), 0); - int pos2 = preferredPositions.getOrDefault(comp2.getType(), 0); - return Integer.compare(pos1, pos2); - } - } -} diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java b/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java deleted file mode 100644 index fc87cbdc644..00000000000 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java +++ /dev/null @@ -1,158 +0,0 @@ -package org.jabref.gui.sidepane; - -import java.util.Collections; -import java.util.List; - -import javafx.scene.Node; -import javafx.scene.control.Button; -import javafx.scene.control.Label; -import javafx.scene.control.Tooltip; -import javafx.scene.layout.BorderPane; -import javafx.scene.layout.HBox; -import javafx.scene.layout.Priority; - -import org.jabref.gui.actions.Action; -import org.jabref.gui.actions.SimpleCommand; -import org.jabref.gui.icon.IconTheme; -import org.jabref.gui.icon.JabRefIcon; -import org.jabref.logic.l10n.Localization; - -@Deprecated -public abstract class SidePaneComponent { - - private final SidePane sidePane; - private final ToggleCommand toggleCommand; - private final JabRefIcon icon; - private final String title; - private Node contentNode; - - public SidePaneComponent(SidePane sidePane, JabRefIcon icon, String title) { - this.sidePane = sidePane; - this.icon = icon; - this.title = title; - this.toggleCommand = new ToggleCommand(this); - } - - protected void hide() { - sidePane.hide(this.getType()); - } - - protected void show() { - sidePane.show(this.getType()); - } - - protected void moveUp() { - sidePane.moveUp(this); - } - - protected void moveDown() { - sidePane.moveDown(this); - } - - /** - * Override this method if the component needs to make any changes before it can close. - */ - public void beforeClosing() { - // Nothing to do by default - } - - /** - * Override this method if the component needs to do any actions after it is shown. - */ - public void afterOpening() { - // Nothing to do by default - } - - /** - * Specifies how to this side pane component behaves if there is additional vertical space. - */ - public abstract Priority getResizePolicy(); - - /** - * @return the command which toggles this {@link SidePaneComponent} - */ - public ToggleCommand getToggleCommand() { - return toggleCommand; - } - - /** - * @return the action to toggle this {@link SidePaneComponent} - */ - public abstract Action getToggleAction(); - - /** - * @return the content of this component - */ - public final Node getContentPane() { - if (contentNode == null) { - contentNode = createContentPane(); - } - - return contentNode; - } - - /** - * @return the header pane for this component - */ - public final Node getHeader() { - Button close = IconTheme.JabRefIcons.CLOSE.asButton(); - close.setTooltip(new Tooltip(Localization.lang("Hide panel"))); - close.setOnAction(event -> hide()); - - Button up = IconTheme.JabRefIcons.UP.asButton(); - up.setTooltip(new Tooltip(Localization.lang("Move panel up"))); - up.setOnAction(event -> moveUp()); - - Button down = IconTheme.JabRefIcons.DOWN.asButton(); - down.setTooltip(new Tooltip(Localization.lang("Move panel down"))); - down.setOnAction(event -> moveDown()); - - final HBox buttonContainer = new HBox(); - buttonContainer.getChildren().addAll(up, down); - buttonContainer.getChildren().addAll(getAdditionalHeaderButtons()); - buttonContainer.getChildren().add(close); - - BorderPane graphic = new BorderPane(); - graphic.setCenter(icon.getGraphicNode()); - - final Label label = new Label(title); - BorderPane container = new BorderPane(); - container.setCenter(label); - container.setRight(buttonContainer); - container.getStyleClass().add("sidePaneComponentHeader"); - - return container; - } - - protected List getAdditionalHeaderButtons() { - return Collections.emptyList(); - } - - /** - * Create the content of this component - * - * @implNote The {@link SidePane} always creates an instance of every side component (e.g., to get the toggle action) - * but we only want to create the content view if the component is shown to save resources. - * This is the reason for the lazy loading. - */ - protected abstract Node createContentPane(); - - /** - * @return the type of this component - */ - public abstract SidePaneType getType(); - - public class ToggleCommand extends SimpleCommand { - - private final SidePaneComponent component; - - public ToggleCommand(SidePaneComponent component) { - this.component = component; - } - - @Override - public void execute() { - sidePane.toggle(component.getType()); - } - } -} From 971203adc997a5c08788bdcdac24b02db249a5ad Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Mon, 1 Nov 2021 23:21:02 +0100 Subject: [PATCH 13/36] Move Vbox.setVgrow() to SidePaneView --- .../java/org/jabref/gui/sidepane/SidePaneContainerView.java | 1 - src/main/java/org/jabref/gui/sidepane/SidePaneView.java | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java index 0e45c0b6154..c6fe0e02851 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java @@ -93,7 +93,6 @@ private void showVisibleSidePanes() { viewModel.getVisiblePanes().forEach(type -> { SidePaneView view = getSidePaneView(type); getChildren().add(view); - VBox.setVgrow(view, view.getResizePolicy()); }); } diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneView.java b/src/main/java/org/jabref/gui/sidepane/SidePaneView.java index 8ef89e0851f..bca727d234d 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneView.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneView.java @@ -3,6 +3,7 @@ import javafx.scene.Node; import javafx.scene.layout.BorderPane; import javafx.scene.layout.Priority; +import javafx.scene.layout.VBox; public class SidePaneView extends BorderPane { private final SidePaneHeaderView sidePaneHeaderView; @@ -20,6 +21,7 @@ private void initView() { getStyleClass().add("sidePaneComponent"); setTop(sidePaneHeaderView); setCenter(content); + VBox.setVgrow(this, getResizePolicy()); } public Priority getResizePolicy() { From 2266a67a98c5d96be744d6b20c2e434f4a8bb1a7 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Tue, 2 Nov 2021 00:18:47 +0100 Subject: [PATCH 14/36] Merge SidePaneHeaderView into SidePaneView --- .../sidepane/GroupsSidePaneHeaderView.java | 56 ---------------- .../gui/sidepane/GroupsSidePaneView.java | 52 +++++++++++++-- .../gui/sidepane/SidePaneContainerView.java | 48 +++----------- .../gui/sidepane/SidePaneHeaderView.java | 58 ---------------- .../org/jabref/gui/sidepane/SidePaneView.java | 66 ++++++++++++++----- .../sidepane/SidePaneViewContentFactory.java | 41 ++++++++++++ 6 files changed, 148 insertions(+), 173 deletions(-) delete mode 100644 src/main/java/org/jabref/gui/sidepane/GroupsSidePaneHeaderView.java delete mode 100644 src/main/java/org/jabref/gui/sidepane/SidePaneHeaderView.java create mode 100644 src/main/java/org/jabref/gui/sidepane/SidePaneViewContentFactory.java diff --git a/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneHeaderView.java b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneHeaderView.java deleted file mode 100644 index 064a5bbf9c6..00000000000 --- a/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneHeaderView.java +++ /dev/null @@ -1,56 +0,0 @@ -package org.jabref.gui.sidepane; - -import javafx.scene.control.Button; - -import org.jabref.gui.DialogService; -import org.jabref.gui.actions.SimpleCommand; -import org.jabref.gui.groups.GroupModeViewModel; -import org.jabref.gui.groups.GroupViewMode; -import org.jabref.gui.icon.IconTheme; -import org.jabref.logic.l10n.Localization; -import org.jabref.preferences.PreferencesService; - -public class GroupsSidePaneHeaderView extends SidePaneHeaderView { - private final PreferencesService preferences; - private final DialogService dialogService; - private final Button intersectionUnionToggle = IconTheme.JabRefIcons.GROUP_INTERSECTION.asButton(); - - public GroupsSidePaneHeaderView(SidePaneType sidePaneType, SimpleCommand closeCommand, SimpleCommand moveUpCommand, SimpleCommand moveDownCommand, PreferencesService preferences, DialogService dialogService) { - super(sidePaneType, closeCommand, moveUpCommand, moveDownCommand); - this.preferences = preferences; - this.dialogService = dialogService; - setupIntersectionUnionToggle(); - } - - private void setupIntersectionUnionToggle() { - addButtonAtPosition(intersectionUnionToggle, 2); - intersectionUnionToggle.setOnAction(event -> new ToggleUnionIntersectionAction().execute()); - } - - public void afterOpening() { - setGraphicsAndTooltipForButton(preferences.getGroupViewMode()); - } - - private void setGraphicsAndTooltipForButton(GroupViewMode mode) { - GroupModeViewModel modeViewModel = new GroupModeViewModel(mode); - intersectionUnionToggle.setGraphic(modeViewModel.getUnionIntersectionGraphic()); - intersectionUnionToggle.setTooltip(modeViewModel.getUnionIntersectionTooltip()); - } - - public class ToggleUnionIntersectionAction extends SimpleCommand { - - @Override - public void execute() { - GroupViewMode mode = preferences.getGroupViewMode(); - - if (mode == GroupViewMode.UNION) { - preferences.setGroupViewMode(GroupViewMode.INTERSECTION); - dialogService.notify(Localization.lang("Group view mode set to intersection")); - } else if (mode == GroupViewMode.INTERSECTION) { - preferences.setGroupViewMode(GroupViewMode.UNION); - dialogService.notify(Localization.lang("Group view mode set to union")); - } - setGraphicsAndTooltipForButton(mode); - } - } -} diff --git a/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneView.java b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneView.java index e5d92a9c189..f61e858719a 100644 --- a/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneView.java +++ b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneView.java @@ -1,14 +1,56 @@ package org.jabref.gui.sidepane; -import javafx.scene.Node; -import javafx.scene.layout.Priority; +import javafx.scene.control.Button; + +import org.jabref.gui.DialogService; +import org.jabref.gui.actions.SimpleCommand; +import org.jabref.gui.groups.GroupModeViewModel; +import org.jabref.gui.groups.GroupViewMode; +import org.jabref.gui.icon.IconTheme; +import org.jabref.logic.l10n.Localization; +import org.jabref.preferences.PreferencesService; public class GroupsSidePaneView extends SidePaneView { - public GroupsSidePaneView(GroupsSidePaneHeaderView sidePaneHeaderView, Node content, Priority resizePolicy) { - super(sidePaneHeaderView, content, resizePolicy); + private final PreferencesService preferences; + private final DialogService dialogService; + private final Button intersectionUnionToggle = IconTheme.JabRefIcons.GROUP_INTERSECTION.asButton(); + + public GroupsSidePaneView(SimpleCommand closeCommand, SimpleCommand moveUpCommand, SimpleCommand moveDownCommand, SidePaneViewContentFactory contentFactory, PreferencesService preferences, DialogService dialogService) { + super(SidePaneType.GROUPS, closeCommand, moveUpCommand, moveDownCommand, contentFactory); + this.preferences = preferences; + this.dialogService = dialogService; + setupIntersectionUnionToggle(); + } + + private void setupIntersectionUnionToggle() { + addExtraButtonToHeader(intersectionUnionToggle, 2); + intersectionUnionToggle.setOnAction(event -> new ToggleUnionIntersectionAction().execute()); } public void afterOpening() { - ((GroupsSidePaneHeaderView) (getSidePaneHeaderView())).afterOpening(); + setGraphicsAndTooltipForButton(preferences.getGroupViewMode()); + } + + private void setGraphicsAndTooltipForButton(GroupViewMode mode) { + GroupModeViewModel modeViewModel = new GroupModeViewModel(mode); + intersectionUnionToggle.setGraphic(modeViewModel.getUnionIntersectionGraphic()); + intersectionUnionToggle.setTooltip(modeViewModel.getUnionIntersectionTooltip()); + } + + private class ToggleUnionIntersectionAction extends SimpleCommand { + + @Override + public void execute() { + GroupViewMode mode = preferences.getGroupViewMode(); + + if (mode == GroupViewMode.UNION) { + preferences.setGroupViewMode(GroupViewMode.INTERSECTION); + dialogService.notify(Localization.lang("Group view mode set to intersection")); + } else if (mode == GroupViewMode.INTERSECTION) { + preferences.setGroupViewMode(GroupViewMode.UNION); + dialogService.notify(Localization.lang("Group view mode set to union")); + } + setGraphicsAndTooltipForButton(mode); + } } } diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java index c6fe0e02851..824a08a890d 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java @@ -6,22 +6,15 @@ import javax.swing.undo.UndoManager; import javafx.beans.property.BooleanProperty; -import javafx.scene.Node; -import javafx.scene.layout.Priority; import javafx.scene.layout.VBox; import org.jabref.gui.DialogService; import org.jabref.gui.StateManager; import org.jabref.gui.actions.SimpleCommand; -import org.jabref.gui.groups.GroupTreeView; -import org.jabref.gui.importer.fetcher.WebSearchPaneView; -import org.jabref.gui.openoffice.OpenOfficePanel; import org.jabref.gui.util.TaskExecutor; import org.jabref.preferences.PreferencesService; import static org.jabref.gui.sidepane.SidePaneType.GROUPS; -import static org.jabref.gui.sidepane.SidePaneType.OPEN_OFFICE; -import static org.jabref.gui.sidepane.SidePaneType.WEB_SEARCH; public class SidePaneContainerView extends VBox { // Don't use this map directly to lookup sidePaneViews, instead use getSidePaneView() for lazy loading @@ -34,6 +27,8 @@ public class SidePaneContainerView extends VBox { private final StateManager stateManager; private final UndoManager undoManager; + private final SidePaneViewContentFactory sidePaneViewContentFactory; + public SidePaneContainerView(PreferencesService preferencesService, TaskExecutor taskExecutor, DialogService dialogService, @@ -44,50 +39,25 @@ public SidePaneContainerView(PreferencesService preferencesService, this.dialogService = dialogService; this.stateManager = stateManager; this.undoManager = undoManager; + this.sidePaneViewContentFactory = new SidePaneViewContentFactory(preferencesService, taskExecutor, dialogService, stateManager, undoManager); this.viewModel = new SidePaneContainerViewModel(preferencesService); preferencesService.getSidePanePreferences().visiblePanes().forEach(this::show); updateView(); } - private SidePaneView getSidePaneView(SidePaneType sidePaneType) { - SidePaneView sidePaneView = sidePaneViewLookup.get(sidePaneType); + private SidePaneView getSidePaneView(SidePaneType sidePane) { + SidePaneView sidePaneView = sidePaneViewLookup.get(sidePane); if (sidePaneView == null) { - sidePaneView = switch (sidePaneType) { - case GROUPS -> createGroupsSidePaneView(); - case WEB_SEARCH -> createWebSearchSidePaneView(); - case OPEN_OFFICE -> createOpenOfficeSidePaneView(); + sidePaneView = switch (sidePane) { + case GROUPS -> new GroupsSidePaneView(new CloseSidePaneAction(sidePane), new MoveUpAction(sidePane), new MoveDownAction(sidePane), sidePaneViewContentFactory, preferencesService, dialogService); + case WEB_SEARCH, OPEN_OFFICE -> new SidePaneView(sidePane, new CloseSidePaneAction(sidePane), new MoveUpAction(sidePane), new MoveDownAction(sidePane), sidePaneViewContentFactory); }; - sidePaneViewLookup.put(sidePaneType, sidePaneView); + sidePaneViewLookup.put(sidePane, sidePaneView); } return sidePaneView; } - private SidePaneView createGroupsSidePaneView() { - GroupsSidePaneHeaderView groupsHeader = new GroupsSidePaneHeaderView(GROUPS, - new CloseSidePaneAction(GROUPS), new MoveUpAction(GROUPS), new MoveDownAction(GROUPS), preferencesService, dialogService); - GroupTreeView groupsContent = new GroupTreeView(taskExecutor, stateManager, preferencesService, dialogService); - - return new GroupsSidePaneView(groupsHeader, groupsContent, Priority.ALWAYS); - } - - private SidePaneView createOpenOfficeSidePaneView() { - Node openOfficePaneContent = new OpenOfficePanel(preferencesService, preferencesService.getOpenOfficePreferences(), - preferencesService.getKeyBindingRepository(), taskExecutor, dialogService, stateManager, undoManager).getContent(); - return createSidePaneView(OPEN_OFFICE, openOfficePaneContent); - } - - private SidePaneView createWebSearchSidePaneView() { - Node webSearchPaneContent = new WebSearchPaneView(preferencesService, dialogService, stateManager); - return createSidePaneView(WEB_SEARCH, webSearchPaneContent); - } - - private SidePaneView createSidePaneView(SidePaneType sidePaneType, Node paneContent) { - SidePaneHeaderView paneHeaderView = new SidePaneHeaderView(sidePaneType, new CloseSidePaneAction(sidePaneType), - new MoveUpAction(sidePaneType), new MoveDownAction(sidePaneType)); - return new SidePaneView(paneHeaderView, paneContent, Priority.NEVER); - } - private void showVisibleSidePanes() { getChildren().clear(); viewModel.getVisiblePanes().forEach(type -> { diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneHeaderView.java b/src/main/java/org/jabref/gui/sidepane/SidePaneHeaderView.java deleted file mode 100644 index 69a620ad45a..00000000000 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneHeaderView.java +++ /dev/null @@ -1,58 +0,0 @@ -package org.jabref.gui.sidepane; - -import javafx.scene.control.Button; -import javafx.scene.control.Label; -import javafx.scene.control.Tooltip; -import javafx.scene.layout.BorderPane; -import javafx.scene.layout.HBox; - -import org.jabref.gui.actions.SimpleCommand; -import org.jabref.gui.icon.IconTheme; -import org.jabref.logic.l10n.Localization; - -public class SidePaneHeaderView extends BorderPane { - private final SidePaneType sidePaneType; - private final SimpleCommand closeCommand; - private final SimpleCommand moveUpCommand; - private final SimpleCommand moveDownCommand; - - private HBox buttonContainer; - - public SidePaneHeaderView(SidePaneType sidePaneType, SimpleCommand closeCommand, SimpleCommand moveUpCommand, SimpleCommand moveDownCommand) { - this.sidePaneType = sidePaneType; - this.closeCommand = closeCommand; - this.moveUpCommand = moveUpCommand; - this.moveDownCommand = moveDownCommand; - initView(); - } - - private void initView() { - Button closeButton = IconTheme.JabRefIcons.CLOSE.asButton(); - closeButton.setTooltip(new Tooltip(Localization.lang("Hide panel"))); - closeButton.setOnAction(e -> closeCommand.execute()); - - Button upButton = IconTheme.JabRefIcons.UP.asButton(); - upButton.setTooltip(new Tooltip(Localization.lang("Move panel up"))); - upButton.setOnAction(e -> moveUpCommand.execute()); - - Button downButton = IconTheme.JabRefIcons.DOWN.asButton(); - downButton.setTooltip(new Tooltip(Localization.lang("Move panel down"))); - downButton.setOnAction(e -> moveDownCommand.execute()); - - this.buttonContainer = new HBox(); - buttonContainer.getChildren().addAll(upButton, downButton, closeButton); - - Label label = new Label(sidePaneType.getTitle()); - setCenter(label); - setRight(buttonContainer); - getStyleClass().add("sidePaneComponentHeader"); - } - - protected void addButtonAtPosition(Button button, int position) { - this.buttonContainer.getChildren().add(position, button); - } - - public SidePaneType getSidePaneType() { - return sidePaneType; - } -} diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneView.java b/src/main/java/org/jabref/gui/sidepane/SidePaneView.java index bca727d234d..ff6979b018d 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneView.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneView.java @@ -1,34 +1,70 @@ package org.jabref.gui.sidepane; import javafx.scene.Node; +import javafx.scene.control.Button; +import javafx.scene.control.Label; +import javafx.scene.control.Tooltip; import javafx.scene.layout.BorderPane; +import javafx.scene.layout.HBox; import javafx.scene.layout.Priority; import javafx.scene.layout.VBox; +import org.jabref.gui.actions.SimpleCommand; +import org.jabref.gui.icon.IconTheme; +import org.jabref.logic.l10n.Localization; + public class SidePaneView extends BorderPane { - private final SidePaneHeaderView sidePaneHeaderView; - private final Node content; - private final Priority resizePolicy; - - public SidePaneView(SidePaneHeaderView sidePaneHeaderView, Node content, Priority resizePolicy) { - this.sidePaneHeaderView = sidePaneHeaderView; - this.content = content; - this.resizePolicy = resizePolicy; + private final SidePaneType sidePaneType; + private final SimpleCommand closeCommand; + private final SimpleCommand moveUpCommand; + private final SimpleCommand moveDownCommand; + private final SidePaneViewContentFactory contentFactory; + + private HBox buttonContainer; + + public SidePaneView(SidePaneType sidePaneType, SimpleCommand closeCommand, SimpleCommand moveUpCommand, SimpleCommand moveDownCommand, SidePaneViewContentFactory contentFactory) { + this.sidePaneType = sidePaneType; + this.closeCommand = closeCommand; + this.moveUpCommand = moveUpCommand; + this.moveDownCommand = moveDownCommand; + this.contentFactory = contentFactory; initView(); } private void initView() { getStyleClass().add("sidePaneComponent"); - setTop(sidePaneHeaderView); - setCenter(content); - VBox.setVgrow(this, getResizePolicy()); + setTop(createHeaderView()); + setCenter(contentFactory.create(sidePaneType)); + VBox.setVgrow(this, sidePaneType == SidePaneType.GROUPS ? Priority.ALWAYS : Priority.NEVER); } - public Priority getResizePolicy() { - return resizePolicy; + private Node createHeaderView() { + Button closeButton = IconTheme.JabRefIcons.CLOSE.asButton(); + closeButton.setTooltip(new Tooltip(Localization.lang("Hide panel"))); + closeButton.setOnAction(e -> closeCommand.execute()); + + Button upButton = IconTheme.JabRefIcons.UP.asButton(); + upButton.setTooltip(new Tooltip(Localization.lang("Move panel up"))); + upButton.setOnAction(e -> moveUpCommand.execute()); + + Button downButton = IconTheme.JabRefIcons.DOWN.asButton(); + downButton.setTooltip(new Tooltip(Localization.lang("Move panel down"))); + downButton.setOnAction(e -> moveDownCommand.execute()); + + this.buttonContainer = new HBox(); + buttonContainer.getChildren().addAll(upButton, downButton, closeButton); + + Label label = new Label(sidePaneType.getTitle()); + + BorderPane headerView = new BorderPane(); + headerView.setCenter(label); + headerView.setRight(buttonContainer); + headerView.getStyleClass().add("sidePaneComponentHeader"); + + return headerView; } - protected SidePaneHeaderView getSidePaneHeaderView() { - return sidePaneHeaderView; + protected void addExtraButtonToHeader(Button button, int position) { + this.buttonContainer.getChildren().add(position, button); } } diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneViewContentFactory.java b/src/main/java/org/jabref/gui/sidepane/SidePaneViewContentFactory.java new file mode 100644 index 00000000000..0ac8da5ac53 --- /dev/null +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneViewContentFactory.java @@ -0,0 +1,41 @@ +package org.jabref.gui.sidepane; + +import javax.swing.undo.UndoManager; + +import javafx.scene.Node; + +import org.jabref.gui.DialogService; +import org.jabref.gui.StateManager; +import org.jabref.gui.groups.GroupTreeView; +import org.jabref.gui.importer.fetcher.WebSearchPaneView; +import org.jabref.gui.openoffice.OpenOfficePanel; +import org.jabref.gui.util.TaskExecutor; +import org.jabref.preferences.PreferencesService; + +public class SidePaneViewContentFactory { + private final PreferencesService preferences; + private final TaskExecutor taskExecutor; + private final DialogService dialogService; + private final StateManager stateManager; + private final UndoManager undoManager; + + public SidePaneViewContentFactory(PreferencesService preferences, + TaskExecutor taskExecutor, + DialogService dialogService, + StateManager stateManager, + UndoManager undoManager) { + this.preferences = preferences; + this.taskExecutor = taskExecutor; + this.dialogService = dialogService; + this.stateManager = stateManager; + this.undoManager = undoManager; + } + + public Node create(SidePaneType sidePaneType) { + return switch (sidePaneType) { + case GROUPS -> new GroupTreeView(taskExecutor, stateManager, preferences, dialogService); + case OPEN_OFFICE -> new OpenOfficePanel(preferences, preferences.getOpenOfficePreferences(), preferences.getKeyBindingRepository(), taskExecutor, dialogService, stateManager, undoManager).getContent(); + case WEB_SEARCH -> new WebSearchPaneView(preferences, dialogService, stateManager); + }; + } +} From 8224cc5c2bbcb6a4b1bfecbdaf1122e405e54f34 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Tue, 2 Nov 2021 02:34:30 +0100 Subject: [PATCH 15/36] Avoid firing removed and added events on ObservableList - Swapping elements in ObservableLists is rather difficult to handle. The problem with this code here is that two events are fired (one for removing, and on for adding). Perhaps the easiest solution is to use the sort method using a custom comparator. Since the list is small we can also be inefficient and do this as follows: create a copy of visiblePanes, swap the elements there and then use this new list as a reference for the comparator --- .../sidepane/SidePaneContainerViewModel.java | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java index ce93a2e7cec..066235f4677 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java @@ -1,7 +1,10 @@ package org.jabref.gui.sidepane; +import java.util.ArrayList; +import java.util.Collections; import java.util.Comparator; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.stream.IntStream; @@ -68,9 +71,7 @@ public boolean moveUp(SidePaneType sidePane) { int currentPosition = visiblePanes.indexOf(sidePane); if (currentPosition > 0) { int newPosition = currentPosition - 1; - visiblePanes.remove(currentPosition); - visiblePanes.add(newPosition, sidePane); - + swap(visiblePanes, currentPosition, newPosition); updatePreferredPositions(); return true; } @@ -86,9 +87,7 @@ public boolean moveDown(SidePaneType sidePane) { int currentPosition = visiblePanes.indexOf(sidePane); if (currentPosition < (visiblePanes.size() - 1)) { int newPosition = currentPosition + 1; - visiblePanes.remove(currentPosition); - visiblePanes.add(newPosition, sidePane); - + swap(visiblePanes, currentPosition, newPosition); updatePreferredPositions(); return true; } @@ -138,6 +137,15 @@ public boolean isSidePaneVisible(SidePaneType sidePane) { return visiblePanes.contains(sidePane); } + /** + * This implementation is inefficient because of some JavaFX limitations, we only advice to use it on small lists + */ + private void swap(ObservableList observableList, int i, int j) { + List placeholder = new ArrayList<>(observableList); + Collections.swap(placeholder, i, j); + observableList.sort(Comparator.comparingInt(placeholder::indexOf)); + } + /** * Helper class for sorting visible side panes based on their preferred position. */ From 5a877ab2befe3d6d2da8be6b929e399599897cb0 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Tue, 2 Nov 2021 23:05:51 +0100 Subject: [PATCH 16/36] Use shorter more declarative code for binding --- .../sidepane/SidePaneContainerViewModel.java | 25 ++++--------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java index 066235f4677..82edd623057 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java @@ -8,10 +8,10 @@ import java.util.Map; import java.util.stream.IntStream; +import javafx.beans.binding.Bindings; import javafx.beans.property.BooleanProperty; import javafx.beans.property.SimpleBooleanProperty; import javafx.collections.FXCollections; -import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; import org.jabref.gui.AbstractViewModel; @@ -28,25 +28,10 @@ public class SidePaneContainerViewModel extends AbstractViewModel { public SidePaneContainerViewModel(PreferencesService preferencesService) { this.preferencesService = preferencesService; - visiblePanes.addListener((ListChangeListener) change -> { - while (change.next()) { - if (change.wasAdded()) { - SidePaneType addedSidePane = change.getAddedSubList().get(0); - switch (addedSidePane) { - case GROUPS -> groupsPaneVisible.setValue(true); - case OPEN_OFFICE -> openOfficePaneVisible.setValue(true); - case WEB_SEARCH -> webSearchPaneVisible.setValue(true); - } - } else if (change.wasRemoved()) { - SidePaneType removedSidePane = change.getRemoved().get(0); - switch (removedSidePane) { - case GROUPS -> groupsPaneVisible.setValue(false); - case OPEN_OFFICE -> openOfficePaneVisible.setValue(false); - case WEB_SEARCH -> webSearchPaneVisible.setValue(false); - } - } - } - }); + + groupsPaneVisible.bind(Bindings.createBooleanBinding(() -> visiblePanes.contains(SidePaneType.GROUPS), visiblePanes)); + openOfficePaneVisible.bind(Bindings.createBooleanBinding(() -> visiblePanes.contains(SidePaneType.OPEN_OFFICE), visiblePanes)); + webSearchPaneVisible.bind(Bindings.createBooleanBinding(() -> visiblePanes.contains(SidePaneType.WEB_SEARCH), visiblePanes)); } /** From 1d35778544280dc54499993848b0962d715dcce5 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sat, 6 Nov 2021 21:02:37 +0100 Subject: [PATCH 17/36] Rename SidePaneView to SidePaneComponent and SidePaneContainerView to SidePane --- src/main/java/org/jabref/gui/JabRefFrame.java | 26 ++++++------ ...View.java => GroupsSidePaneComponent.java} | 4 +- ...dePaneContainerView.java => SidePane.java} | 42 +++++++++---------- ...dePaneView.java => SidePaneComponent.java} | 6 +-- ...ctory.java => SidePaneContentFactory.java} | 12 +++--- ...rViewModel.java => SidePaneViewModel.java} | 4 +- 6 files changed, 47 insertions(+), 47 deletions(-) rename src/main/java/org/jabref/gui/sidepane/{GroupsSidePaneView.java => GroupsSidePaneComponent.java} (88%) rename src/main/java/org/jabref/gui/sidepane/{SidePaneContainerView.java => SidePane.java} (71%) rename src/main/java/org/jabref/gui/sidepane/{SidePaneView.java => SidePaneComponent.java} (89%) rename src/main/java/org/jabref/gui/sidepane/{SidePaneViewContentFactory.java => SidePaneContentFactory.java} (78%) rename src/main/java/org/jabref/gui/sidepane/{SidePaneContainerViewModel.java => SidePaneViewModel.java} (97%) diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index 036cae4cf4f..dbe464bb4bd 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -112,7 +112,7 @@ import org.jabref.gui.search.RebuildFulltextSearchIndexAction; import org.jabref.gui.shared.ConnectToSharedDatabaseCommand; import org.jabref.gui.shared.PullChangesFromSharedAction; -import org.jabref.gui.sidepane.SidePaneContainerView; +import org.jabref.gui.sidepane.SidePane; import org.jabref.gui.sidepane.SidePaneType; import org.jabref.gui.slr.ExistingStudySearchAction; import org.jabref.gui.slr.StartNewStudyAction; @@ -173,7 +173,7 @@ public class JabRefFrame extends BorderPane { private final CountingUndoManager undoManager; private final PushToApplicationsManager pushToApplicationsManager; private final DialogService dialogService; - private SidePaneContainerView sidePaneContainerView; + private SidePane sidePane; private TabPane tabbedPane; private PopOver progressViewPopOver; private PopOver entryFromIdPopOver; @@ -429,8 +429,8 @@ private void initLayout() { head.setSpacing(0d); setTop(head); - splitPane.getItems().addAll(sidePaneContainerView, tabbedPane); - SplitPane.setResizableWithParent(sidePaneContainerView, false); + splitPane.getItems().addAll(sidePane, tabbedPane); + SplitPane.setResizableWithParent(sidePane, false); // We need to wait with setting the divider since it gets reset a few times during the initial set-up mainStage.showingProperty().addListener(new ChangeListener<>() { @@ -439,14 +439,14 @@ private void initLayout() { public void changed(ObservableValue observable, Boolean oldValue, Boolean showing) { if (showing) { setDividerPosition(); - EasyBind.subscribe(sidePaneContainerView.visibleProperty(), visible -> { + EasyBind.subscribe(sidePane.visibleProperty(), visible -> { if (visible) { - if (!splitPane.getItems().contains(sidePaneContainerView)) { - splitPane.getItems().add(0, sidePaneContainerView); + if (!splitPane.getItems().contains(sidePane)) { + splitPane.getItems().add(0, sidePane); setDividerPosition(); } } else { - splitPane.getItems().remove(sidePaneContainerView); + splitPane.getItems().remove(sidePane); } }); @@ -571,7 +571,7 @@ public void showLibraryTab(LibraryTab libraryTab) { } public void init() { - sidePaneContainerView = new SidePaneContainerView(prefs, taskExecutor, dialogService, stateManager, undoManager); + sidePane = new SidePane(prefs, taskExecutor, dialogService, stateManager, undoManager); tabbedPane = new TabPane(); tabbedPane.setTabDragPolicy(TabPane.TabDragPolicy.REORDER); @@ -836,9 +836,9 @@ private MenuBar createMenu() { ); view.getItems().addAll( - createSidePaneCheckMenuItem(sidePaneContainerView, factory, SidePaneType.WEB_SEARCH), - createSidePaneCheckMenuItem(sidePaneContainerView, factory, SidePaneType.GROUPS), - createSidePaneCheckMenuItem(sidePaneContainerView, factory, SidePaneType.OPEN_OFFICE), + createSidePaneCheckMenuItem(sidePane, factory, SidePaneType.WEB_SEARCH), + createSidePaneCheckMenuItem(sidePane, factory, SidePaneType.GROUPS), + createSidePaneCheckMenuItem(sidePane, factory, SidePaneType.OPEN_OFFICE), new SeparatorMenuItem(), @@ -935,7 +935,7 @@ private Button createNewEntryFromIdButton() { return newEntryFromIdButton; } - private CheckMenuItem createSidePaneCheckMenuItem(SidePaneContainerView container, ActionFactory factory, SidePaneType sidePane) { + private CheckMenuItem createSidePaneCheckMenuItem(SidePane container, ActionFactory factory, SidePaneType sidePane) { CheckMenuItem checkMenuItem = factory.createCheckMenuItem(sidePane.getToggleAction(), container.getToggleCommandFor(sidePane), container.sidePaneVisibleProperty(sidePane).get()); EasyBind.subscribe(container.sidePaneVisibleProperty(sidePane), checkMenuItem::setSelected); return checkMenuItem; diff --git a/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneView.java b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneComponent.java similarity index 88% rename from src/main/java/org/jabref/gui/sidepane/GroupsSidePaneView.java rename to src/main/java/org/jabref/gui/sidepane/GroupsSidePaneComponent.java index f61e858719a..695916854ad 100644 --- a/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneView.java +++ b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneComponent.java @@ -10,12 +10,12 @@ import org.jabref.logic.l10n.Localization; import org.jabref.preferences.PreferencesService; -public class GroupsSidePaneView extends SidePaneView { +public class GroupsSidePaneComponent extends SidePaneComponent { private final PreferencesService preferences; private final DialogService dialogService; private final Button intersectionUnionToggle = IconTheme.JabRefIcons.GROUP_INTERSECTION.asButton(); - public GroupsSidePaneView(SimpleCommand closeCommand, SimpleCommand moveUpCommand, SimpleCommand moveDownCommand, SidePaneViewContentFactory contentFactory, PreferencesService preferences, DialogService dialogService) { + public GroupsSidePaneComponent(SimpleCommand closeCommand, SimpleCommand moveUpCommand, SimpleCommand moveDownCommand, SidePaneContentFactory contentFactory, PreferencesService preferences, DialogService dialogService) { super(SidePaneType.GROUPS, closeCommand, moveUpCommand, moveDownCommand, contentFactory); this.preferences = preferences; this.dialogService = dialogService; diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java b/src/main/java/org/jabref/gui/sidepane/SidePane.java similarity index 71% rename from src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java rename to src/main/java/org/jabref/gui/sidepane/SidePane.java index 824a08a890d..9eb3b929a71 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerView.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePane.java @@ -16,10 +16,10 @@ import static org.jabref.gui.sidepane.SidePaneType.GROUPS; -public class SidePaneContainerView extends VBox { +public class SidePane extends VBox { // Don't use this map directly to lookup sidePaneViews, instead use getSidePaneView() for lazy loading - private final Map sidePaneViewLookup = new HashMap<>(); - private final SidePaneContainerViewModel viewModel; + private final Map sidePaneComponentLookup = new HashMap<>(); + private final SidePaneViewModel viewModel; private final PreferencesService preferencesService; private final TaskExecutor taskExecutor; @@ -27,41 +27,41 @@ public class SidePaneContainerView extends VBox { private final StateManager stateManager; private final UndoManager undoManager; - private final SidePaneViewContentFactory sidePaneViewContentFactory; + private final SidePaneContentFactory sidePaneContentFactory; - public SidePaneContainerView(PreferencesService preferencesService, - TaskExecutor taskExecutor, - DialogService dialogService, - StateManager stateManager, - UndoManager undoManager) { + public SidePane(PreferencesService preferencesService, + TaskExecutor taskExecutor, + DialogService dialogService, + StateManager stateManager, + UndoManager undoManager) { this.preferencesService = preferencesService; this.taskExecutor = taskExecutor; this.dialogService = dialogService; this.stateManager = stateManager; this.undoManager = undoManager; - this.sidePaneViewContentFactory = new SidePaneViewContentFactory(preferencesService, taskExecutor, dialogService, stateManager, undoManager); - this.viewModel = new SidePaneContainerViewModel(preferencesService); + this.sidePaneContentFactory = new SidePaneContentFactory(preferencesService, taskExecutor, dialogService, stateManager, undoManager); + this.viewModel = new SidePaneViewModel(preferencesService); preferencesService.getSidePanePreferences().visiblePanes().forEach(this::show); updateView(); } - private SidePaneView getSidePaneView(SidePaneType sidePane) { - SidePaneView sidePaneView = sidePaneViewLookup.get(sidePane); - if (sidePaneView == null) { - sidePaneView = switch (sidePane) { - case GROUPS -> new GroupsSidePaneView(new CloseSidePaneAction(sidePane), new MoveUpAction(sidePane), new MoveDownAction(sidePane), sidePaneViewContentFactory, preferencesService, dialogService); - case WEB_SEARCH, OPEN_OFFICE -> new SidePaneView(sidePane, new CloseSidePaneAction(sidePane), new MoveUpAction(sidePane), new MoveDownAction(sidePane), sidePaneViewContentFactory); + private SidePaneComponent getSidePaneComponent(SidePaneType sidePane) { + SidePaneComponent sidePaneComponent = sidePaneComponentLookup.get(sidePane); + if (sidePaneComponent == null) { + sidePaneComponent = switch (sidePane) { + case GROUPS -> new GroupsSidePaneComponent(new CloseSidePaneAction(sidePane), new MoveUpAction(sidePane), new MoveDownAction(sidePane), sidePaneContentFactory, preferencesService, dialogService); + case WEB_SEARCH, OPEN_OFFICE -> new SidePaneComponent(sidePane, new CloseSidePaneAction(sidePane), new MoveUpAction(sidePane), new MoveDownAction(sidePane), sidePaneContentFactory); }; - sidePaneViewLookup.put(sidePane, sidePaneView); + sidePaneComponentLookup.put(sidePane, sidePaneComponent); } - return sidePaneView; + return sidePaneComponent; } private void showVisibleSidePanes() { getChildren().clear(); viewModel.getVisiblePanes().forEach(type -> { - SidePaneView view = getSidePaneView(type); + SidePaneComponent view = getSidePaneComponent(type); getChildren().add(view); }); } @@ -70,7 +70,7 @@ private void show(SidePaneType sidePane) { if (viewModel.show(sidePane)) { updateView(); if (sidePane == GROUPS) { - ((GroupsSidePaneView) getSidePaneView(sidePane)).afterOpening(); + ((GroupsSidePaneComponent) getSidePaneComponent(sidePane)).afterOpening(); } } } diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneView.java b/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java similarity index 89% rename from src/main/java/org/jabref/gui/sidepane/SidePaneView.java rename to src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java index ff6979b018d..02c73857e74 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneView.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java @@ -13,16 +13,16 @@ import org.jabref.gui.icon.IconTheme; import org.jabref.logic.l10n.Localization; -public class SidePaneView extends BorderPane { +public class SidePaneComponent extends BorderPane { private final SidePaneType sidePaneType; private final SimpleCommand closeCommand; private final SimpleCommand moveUpCommand; private final SimpleCommand moveDownCommand; - private final SidePaneViewContentFactory contentFactory; + private final SidePaneContentFactory contentFactory; private HBox buttonContainer; - public SidePaneView(SidePaneType sidePaneType, SimpleCommand closeCommand, SimpleCommand moveUpCommand, SimpleCommand moveDownCommand, SidePaneViewContentFactory contentFactory) { + public SidePaneComponent(SidePaneType sidePaneType, SimpleCommand closeCommand, SimpleCommand moveUpCommand, SimpleCommand moveDownCommand, SidePaneContentFactory contentFactory) { this.sidePaneType = sidePaneType; this.closeCommand = closeCommand; this.moveUpCommand = moveUpCommand; diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneViewContentFactory.java b/src/main/java/org/jabref/gui/sidepane/SidePaneContentFactory.java similarity index 78% rename from src/main/java/org/jabref/gui/sidepane/SidePaneViewContentFactory.java rename to src/main/java/org/jabref/gui/sidepane/SidePaneContentFactory.java index 0ac8da5ac53..bc32980765a 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneViewContentFactory.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneContentFactory.java @@ -12,18 +12,18 @@ import org.jabref.gui.util.TaskExecutor; import org.jabref.preferences.PreferencesService; -public class SidePaneViewContentFactory { +public class SidePaneContentFactory { private final PreferencesService preferences; private final TaskExecutor taskExecutor; private final DialogService dialogService; private final StateManager stateManager; private final UndoManager undoManager; - public SidePaneViewContentFactory(PreferencesService preferences, - TaskExecutor taskExecutor, - DialogService dialogService, - StateManager stateManager, - UndoManager undoManager) { + public SidePaneContentFactory(PreferencesService preferences, + TaskExecutor taskExecutor, + DialogService dialogService, + StateManager stateManager, + UndoManager undoManager) { this.preferences = preferences; this.taskExecutor = taskExecutor; this.dialogService = dialogService; diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java similarity index 97% rename from src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java rename to src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java index 82edd623057..b80579d0717 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneContainerViewModel.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java @@ -17,7 +17,7 @@ import org.jabref.gui.AbstractViewModel; import org.jabref.preferences.PreferencesService; -public class SidePaneContainerViewModel extends AbstractViewModel { +public class SidePaneViewModel extends AbstractViewModel { private final PreferencesService preferencesService; // TODO('Use preferencesService.getSidePanePreferences().visiblePanes() as the single source of truth') private final ObservableList visiblePanes = FXCollections.observableArrayList(); @@ -26,7 +26,7 @@ public class SidePaneContainerViewModel extends AbstractViewModel { private final BooleanProperty openOfficePaneVisible = new SimpleBooleanProperty(); private final BooleanProperty webSearchPaneVisible = new SimpleBooleanProperty(); - public SidePaneContainerViewModel(PreferencesService preferencesService) { + public SidePaneViewModel(PreferencesService preferencesService) { this.preferencesService = preferencesService; groupsPaneVisible.bind(Bindings.createBooleanBinding(() -> visiblePanes.contains(SidePaneType.GROUPS), visiblePanes)); From 6d9d52dd02144801f5d502a9e52be62405253b40 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sat, 6 Nov 2021 21:09:15 +0100 Subject: [PATCH 18/36] Rename initView() to initialize() --- .../org/jabref/gui/importer/fetcher/WebSearchPaneView.java | 4 ++-- src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneView.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneView.java index 5f4b25269c2..fb4f9352850 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneView.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneView.java @@ -35,10 +35,10 @@ public class WebSearchPaneView extends VBox { public WebSearchPaneView(PreferencesService preferences, DialogService dialogService, StateManager stateManager) { this.preferences = preferences; this.viewModel = new WebSearchPaneViewModel(preferences, dialogService, stateManager); - initView(); + initialize(); } - private void initView() { + private void initialize() { ComboBox fetchers = new ComboBox<>(); new ViewModelListCellFactory() .withText(SearchBasedFetcher::getName) diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java b/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java index 02c73857e74..5580ecba8cb 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java @@ -28,10 +28,10 @@ public SidePaneComponent(SidePaneType sidePaneType, SimpleCommand closeCommand, this.moveUpCommand = moveUpCommand; this.moveDownCommand = moveDownCommand; this.contentFactory = contentFactory; - initView(); + initialize(); } - private void initView() { + private void initialize() { getStyleClass().add("sidePaneComponent"); setTop(createHeaderView()); setCenter(contentFactory.create(sidePaneType)); From 2f0590d2f78bea01c412c26844baa09b71ae24f5 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sat, 6 Nov 2021 21:21:45 +0100 Subject: [PATCH 19/36] Add an overloading of createCheckMenuItem() with a selected property --- src/main/java/org/jabref/gui/JabRefFrame.java | 14 +++++++------- .../java/org/jabref/gui/actions/ActionFactory.java | 10 ++++++++++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index dbe464bb4bd..caa0494ce60 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -834,11 +834,13 @@ private MenuBar createMenu() { factory.createMenuItem(StandardActions.REBUILD_FULLTEXT_SEARCH_INDEX, new RebuildFulltextSearchIndexAction(stateManager, this::getCurrentLibraryTab, dialogService, prefs.getFilePreferences())) ); - + SidePaneType webSearchPane = SidePaneType.WEB_SEARCH; + SidePaneType groupsPane = SidePaneType.GROUPS; + SidePaneType openOfficePane = SidePaneType.OPEN_OFFICE; view.getItems().addAll( - createSidePaneCheckMenuItem(sidePane, factory, SidePaneType.WEB_SEARCH), - createSidePaneCheckMenuItem(sidePane, factory, SidePaneType.GROUPS), - createSidePaneCheckMenuItem(sidePane, factory, SidePaneType.OPEN_OFFICE), + factory.createCheckMenuItem(webSearchPane.getToggleAction(), sidePane.getToggleCommandFor(webSearchPane), sidePane.sidePaneVisibleProperty(webSearchPane)), + factory.createCheckMenuItem(groupsPane.getToggleAction(), sidePane.getToggleCommandFor(groupsPane), sidePane.sidePaneVisibleProperty(groupsPane)), + factory.createCheckMenuItem(openOfficePane.getToggleAction(), sidePane.getToggleCommandFor(openOfficePane), sidePane.sidePaneVisibleProperty(openOfficePane)), new SeparatorMenuItem(), @@ -936,9 +938,7 @@ private Button createNewEntryFromIdButton() { } private CheckMenuItem createSidePaneCheckMenuItem(SidePane container, ActionFactory factory, SidePaneType sidePane) { - CheckMenuItem checkMenuItem = factory.createCheckMenuItem(sidePane.getToggleAction(), container.getToggleCommandFor(sidePane), container.sidePaneVisibleProperty(sidePane).get()); - EasyBind.subscribe(container.sidePaneVisibleProperty(sidePane), checkMenuItem::setSelected); - return checkMenuItem; + return factory.createCheckMenuItem(sidePane.getToggleAction(), container.getToggleCommandFor(sidePane), container.sidePaneVisibleProperty(sidePane)); } private Group createTaskIndicator() { diff --git a/src/main/java/org/jabref/gui/actions/ActionFactory.java b/src/main/java/org/jabref/gui/actions/ActionFactory.java index 151a596b66c..9be7a2c63d0 100644 --- a/src/main/java/org/jabref/gui/actions/ActionFactory.java +++ b/src/main/java/org/jabref/gui/actions/ActionFactory.java @@ -5,6 +5,7 @@ import java.lang.reflect.Method; import java.util.Objects; +import javafx.beans.property.BooleanProperty; import javafx.scene.control.Button; import javafx.scene.control.ButtonBase; import javafx.scene.control.CheckMenuItem; @@ -113,6 +114,15 @@ public CheckMenuItem createCheckMenuItem(Action action, Command command, boolean return checkMenuItem; } + public CheckMenuItem createCheckMenuItem(Action action, Command command, BooleanProperty selectedProperty) { + CheckMenuItem checkMenuItem = ActionUtils.createCheckMenuItem(new JabRefAction(action, command, keyBindingRepository, Sources.FromMenu)); + checkMenuItem.setSelected(selectedProperty.getValue()); + EasyBind.subscribe(selectedProperty, checkMenuItem::setSelected); + setGraphic(checkMenuItem, action); + + return checkMenuItem; + } + public Menu createMenu(Action action) { Menu menu = ActionUtils.createMenu(new JabRefAction(action, keyBindingRepository)); From 58edb1c36787ab739dd9b169e6cffdc5e179865c Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sat, 6 Nov 2021 21:24:10 +0100 Subject: [PATCH 20/36] Rename sidePaneVisibleProperty() to paneVisibleProperty() --- src/main/java/org/jabref/gui/JabRefFrame.java | 8 ++++---- src/main/java/org/jabref/gui/sidepane/SidePane.java | 8 ++++---- .../java/org/jabref/gui/sidepane/SidePaneViewModel.java | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index caa0494ce60..9c7505c05bf 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -838,9 +838,9 @@ private MenuBar createMenu() { SidePaneType groupsPane = SidePaneType.GROUPS; SidePaneType openOfficePane = SidePaneType.OPEN_OFFICE; view.getItems().addAll( - factory.createCheckMenuItem(webSearchPane.getToggleAction(), sidePane.getToggleCommandFor(webSearchPane), sidePane.sidePaneVisibleProperty(webSearchPane)), - factory.createCheckMenuItem(groupsPane.getToggleAction(), sidePane.getToggleCommandFor(groupsPane), sidePane.sidePaneVisibleProperty(groupsPane)), - factory.createCheckMenuItem(openOfficePane.getToggleAction(), sidePane.getToggleCommandFor(openOfficePane), sidePane.sidePaneVisibleProperty(openOfficePane)), + factory.createCheckMenuItem(webSearchPane.getToggleAction(), sidePane.getToggleCommandFor(webSearchPane), sidePane.paneVisibleProperty(webSearchPane)), + factory.createCheckMenuItem(groupsPane.getToggleAction(), sidePane.getToggleCommandFor(groupsPane), sidePane.paneVisibleProperty(groupsPane)), + factory.createCheckMenuItem(openOfficePane.getToggleAction(), sidePane.getToggleCommandFor(openOfficePane), sidePane.paneVisibleProperty(openOfficePane)), new SeparatorMenuItem(), @@ -938,7 +938,7 @@ private Button createNewEntryFromIdButton() { } private CheckMenuItem createSidePaneCheckMenuItem(SidePane container, ActionFactory factory, SidePaneType sidePane) { - return factory.createCheckMenuItem(sidePane.getToggleAction(), container.getToggleCommandFor(sidePane), container.sidePaneVisibleProperty(sidePane)); + return factory.createCheckMenuItem(sidePane.getToggleAction(), container.getToggleCommandFor(sidePane), container.paneVisibleProperty(sidePane)); } private Group createTaskIndicator() { diff --git a/src/main/java/org/jabref/gui/sidepane/SidePane.java b/src/main/java/org/jabref/gui/sidepane/SidePane.java index 9eb3b929a71..37aa7e1e527 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePane.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePane.java @@ -109,10 +109,10 @@ private void updateView() { setVisible(!viewModel.getVisiblePanes().isEmpty()); } - public BooleanProperty sidePaneVisibleProperty(SidePaneType sidePane) { - return switch (sidePane) { - case GROUPS -> viewModel.groupsSidePaneVisibleProperty(); - case WEB_SEARCH -> viewModel.webSearchSidePaneVisibleProperty(); + public BooleanProperty paneVisibleProperty(SidePaneType pane) { + return switch (pane) { + case GROUPS -> viewModel.groupsPaneVisibleProperty(); + case WEB_SEARCH -> viewModel.webSearchPaneVisibleProperty(); case OPEN_OFFICE -> viewModel.openOfficePaneVisibleProperty(); }; } diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java index b80579d0717..98b7318f8dc 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java @@ -106,7 +106,7 @@ public boolean hide(SidePaneType sidePane) { } } - public BooleanProperty groupsSidePaneVisibleProperty() { + public BooleanProperty groupsPaneVisibleProperty() { return groupsPaneVisible; } @@ -114,7 +114,7 @@ public BooleanProperty openOfficePaneVisibleProperty() { return openOfficePaneVisible; } - public BooleanProperty webSearchSidePaneVisibleProperty() { + public BooleanProperty webSearchPaneVisibleProperty() { return webSearchPaneVisible; } From 7e70c256cd781d6d2d86b4e2049d566f14f328b4 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sat, 6 Nov 2021 21:32:24 +0100 Subject: [PATCH 21/36] Use the name pane rather than sidePane to call components in the SidePane --- .../org/jabref/gui/sidepane/SidePane.java | 78 +++++++++---------- .../gui/sidepane/SidePaneViewModel.java | 40 +++++----- 2 files changed, 59 insertions(+), 59 deletions(-) diff --git a/src/main/java/org/jabref/gui/sidepane/SidePane.java b/src/main/java/org/jabref/gui/sidepane/SidePane.java index 37aa7e1e527..ddbe4c88449 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePane.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePane.java @@ -46,19 +46,19 @@ public SidePane(PreferencesService preferencesService, updateView(); } - private SidePaneComponent getSidePaneComponent(SidePaneType sidePane) { - SidePaneComponent sidePaneComponent = sidePaneComponentLookup.get(sidePane); + private SidePaneComponent getSidePaneComponent(SidePaneType pane) { + SidePaneComponent sidePaneComponent = sidePaneComponentLookup.get(pane); if (sidePaneComponent == null) { - sidePaneComponent = switch (sidePane) { - case GROUPS -> new GroupsSidePaneComponent(new CloseSidePaneAction(sidePane), new MoveUpAction(sidePane), new MoveDownAction(sidePane), sidePaneContentFactory, preferencesService, dialogService); - case WEB_SEARCH, OPEN_OFFICE -> new SidePaneComponent(sidePane, new CloseSidePaneAction(sidePane), new MoveUpAction(sidePane), new MoveDownAction(sidePane), sidePaneContentFactory); + sidePaneComponent = switch (pane) { + case GROUPS -> new GroupsSidePaneComponent(new ClosePaneAction(pane), new MoveUpAction(pane), new MoveDownAction(pane), sidePaneContentFactory, preferencesService, dialogService); + case WEB_SEARCH, OPEN_OFFICE -> new SidePaneComponent(pane, new ClosePaneAction(pane), new MoveUpAction(pane), new MoveDownAction(pane), sidePaneContentFactory); }; - sidePaneComponentLookup.put(sidePane, sidePaneComponent); + sidePaneComponentLookup.put(pane, sidePaneComponent); } return sidePaneComponent; } - private void showVisibleSidePanes() { + private void showVisiblePanes() { getChildren().clear(); viewModel.getVisiblePanes().forEach(type -> { SidePaneComponent view = getSidePaneComponent(type); @@ -66,29 +66,29 @@ private void showVisibleSidePanes() { }); } - private void show(SidePaneType sidePane) { - if (viewModel.show(sidePane)) { + private void show(SidePaneType pane) { + if (viewModel.show(pane)) { updateView(); - if (sidePane == GROUPS) { - ((GroupsSidePaneComponent) getSidePaneComponent(sidePane)).afterOpening(); + if (pane == GROUPS) { + ((GroupsSidePaneComponent) getSidePaneComponent(pane)).afterOpening(); } } } - private void hide(SidePaneType sidePane) { - if (viewModel.hide(sidePane)) { + private void hide(SidePaneType pane) { + if (viewModel.hide(pane)) { updateView(); } } - private void moveUp(SidePaneType sidePane) { - if (viewModel.moveUp(sidePane)) { + private void moveUp(SidePaneType pane) { + if (viewModel.moveUp(pane)) { updateView(); } } - private void moveDown(SidePaneType sidePane) { - if (viewModel.moveDown(sidePane)) { + private void moveDown(SidePaneType pane) { + if (viewModel.moveDown(pane)) { updateView(); } } @@ -96,16 +96,16 @@ private void moveDown(SidePaneType sidePane) { /** * If the given component is visible it will be hidden and the other way around. */ - private void toggle(SidePaneType sidePane) { - if (viewModel.isSidePaneVisible(sidePane)) { - hide(sidePane); + private void toggle(SidePaneType pane) { + if (viewModel.isPaneVisible(pane)) { + hide(pane); } else { - show(sidePane); + show(pane); } } private void updateView() { - showVisibleSidePanes(); + showVisiblePanes(); setVisible(!viewModel.getVisiblePanes().isEmpty()); } @@ -121,56 +121,56 @@ public ToggleCommand getToggleCommandFor(SidePaneType sidePane) { return new ToggleCommand(sidePane); } - private class CloseSidePaneAction extends SimpleCommand { - private final SidePaneType toCloseSidePane; + private class ClosePaneAction extends SimpleCommand { + private final SidePaneType toClosePane; - public CloseSidePaneAction(SidePaneType toCloseSidePane) { - this.toCloseSidePane = toCloseSidePane; + public ClosePaneAction(SidePaneType toClosePane) { + this.toClosePane = toClosePane; } @Override public void execute() { - hide(toCloseSidePane); + hide(toClosePane); } } private class MoveUpAction extends SimpleCommand { - private final SidePaneType toMoveUpSidePane; + private final SidePaneType toMoveUpPane; - public MoveUpAction(SidePaneType toMoveUpSidePane) { - this.toMoveUpSidePane = toMoveUpSidePane; + public MoveUpAction(SidePaneType toMoveUpPane) { + this.toMoveUpPane = toMoveUpPane; } @Override public void execute() { - moveUp(toMoveUpSidePane); + moveUp(toMoveUpPane); } } private class MoveDownAction extends SimpleCommand { - private final SidePaneType toMoveDownSidePane; + private final SidePaneType toMoveDownPane; - public MoveDownAction(SidePaneType toMoveDownSidePane) { - this.toMoveDownSidePane = toMoveDownSidePane; + public MoveDownAction(SidePaneType toMoveDownPane) { + this.toMoveDownPane = toMoveDownPane; } @Override public void execute() { - moveDown(toMoveDownSidePane); + moveDown(toMoveDownPane); } } public class ToggleCommand extends SimpleCommand { - private final SidePaneType sidePane; + private final SidePaneType pane; - public ToggleCommand(SidePaneType sidePane) { - this.sidePane = sidePane; + public ToggleCommand(SidePaneType pane) { + this.pane = pane; } @Override public void execute() { - toggle(sidePane); + toggle(pane); } } } diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java index 98b7318f8dc..807d897c4e1 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java @@ -49,11 +49,11 @@ public ObservableList getVisiblePanes() { } /** - * @return True if sidePane is visible, and it can still move up therefore we should update the view + * @return True if pane is visible, and it can still move up therefore we should update the view */ - public boolean moveUp(SidePaneType sidePane) { - if (visiblePanes.contains(sidePane)) { - int currentPosition = visiblePanes.indexOf(sidePane); + public boolean moveUp(SidePaneType pane) { + if (visiblePanes.contains(pane)) { + int currentPosition = visiblePanes.indexOf(pane); if (currentPosition > 0) { int newPosition = currentPosition - 1; swap(visiblePanes, currentPosition, newPosition); @@ -65,11 +65,11 @@ public boolean moveUp(SidePaneType sidePane) { } /** - * @return True if sidePane is visible, and it can still move down therefore we should update the view + * @return True if pane is visible, and it can still move down therefore we should update the view */ - public boolean moveDown(SidePaneType sidePane) { - if (visiblePanes.contains(sidePane)) { - int currentPosition = visiblePanes.indexOf(sidePane); + public boolean moveDown(SidePaneType pane) { + if (visiblePanes.contains(pane)) { + int currentPosition = visiblePanes.indexOf(pane); if (currentPosition < (visiblePanes.size() - 1)) { int newPosition = currentPosition + 1; swap(visiblePanes, currentPosition, newPosition); @@ -81,12 +81,12 @@ public boolean moveDown(SidePaneType sidePane) { } /** - * @return True if sidePane is not already shown which means the view needs to be updated + * @return True if pane is not already shown which means the view needs to be updated */ - public boolean show(SidePaneType sidePane) { - if (!visiblePanes.contains(sidePane)) { - visiblePanes.add(sidePane); - preferencesService.getSidePanePreferences().visiblePanes().add(sidePane); + public boolean show(SidePaneType pane) { + if (!visiblePanes.contains(pane)) { + visiblePanes.add(pane); + preferencesService.getSidePanePreferences().visiblePanes().add(pane); visiblePanes.sorted(new PreferredIndexSort(preferencesService)); return true; } @@ -94,12 +94,12 @@ public boolean show(SidePaneType sidePane) { } /** - * @return True if sidePane is visible which means the view needs to be updated + * @return True if pane is visible which means the view needs to be updated */ - public boolean hide(SidePaneType sidePane) { - if (visiblePanes.contains(sidePane)) { - visiblePanes.remove(sidePane); - preferencesService.getSidePanePreferences().visiblePanes().remove(sidePane); + public boolean hide(SidePaneType pane) { + if (visiblePanes.contains(pane)) { + visiblePanes.remove(pane); + preferencesService.getSidePanePreferences().visiblePanes().remove(pane); return true; } else { return false; @@ -118,8 +118,8 @@ public BooleanProperty webSearchPaneVisibleProperty() { return webSearchPaneVisible; } - public boolean isSidePaneVisible(SidePaneType sidePane) { - return visiblePanes.contains(sidePane); + public boolean isPaneVisible(SidePaneType pane) { + return visiblePanes.contains(pane); } /** From 9cd889476dc9321b71d170013da13b97983b7067 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sat, 6 Nov 2021 21:38:57 +0100 Subject: [PATCH 22/36] Remove unused method createSidePaneCheckMenuItem() --- src/main/java/org/jabref/gui/JabRefFrame.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index 9c7505c05bf..05aa4806437 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -937,10 +937,6 @@ private Button createNewEntryFromIdButton() { return newEntryFromIdButton; } - private CheckMenuItem createSidePaneCheckMenuItem(SidePane container, ActionFactory factory, SidePaneType sidePane) { - return factory.createCheckMenuItem(sidePane.getToggleAction(), container.getToggleCommandFor(sidePane), container.paneVisibleProperty(sidePane)); - } - private Group createTaskIndicator() { ProgressIndicator indicator = new ProgressIndicator(); indicator.getStyleClass().add("progress-indicatorToolbar"); From 717686227a7cd2309850485452fdc126a6f845e4 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sun, 7 Nov 2021 00:28:45 +0100 Subject: [PATCH 23/36] Share the visibility property of side pane components by moving it to StateManager --- .../java/org/jabref/gui/StateManager.java | 40 ++++++++++++ .../org/jabref/gui/sidepane/SidePane.java | 24 +++++-- .../gui/sidepane/SidePaneViewModel.java | 63 ++++++------------- 3 files changed, 77 insertions(+), 50 deletions(-) diff --git a/src/main/java/org/jabref/gui/StateManager.java b/src/main/java/org/jabref/gui/StateManager.java index fb6afbd5a30..40bba82a6b4 100644 --- a/src/main/java/org/jabref/gui/StateManager.java +++ b/src/main/java/org/jabref/gui/StateManager.java @@ -1,22 +1,30 @@ package org.jabref.gui; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; +import java.util.stream.IntStream; +import javafx.beans.InvalidationListener; import javafx.beans.Observable; import javafx.beans.binding.Bindings; +import javafx.beans.property.BooleanProperty; import javafx.beans.property.IntegerProperty; import javafx.beans.property.ReadOnlyListProperty; import javafx.beans.property.ReadOnlyListWrapper; +import javafx.beans.property.SimpleBooleanProperty; import javafx.beans.property.SimpleIntegerProperty; import javafx.collections.FXCollections; +import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; import javafx.collections.ObservableMap; import javafx.concurrent.Task; import javafx.scene.Node; +import org.jabref.gui.sidepane.SidePaneType; import org.jabref.gui.util.CustomLocalDragboard; import org.jabref.gui.util.DialogWindowState; import org.jabref.gui.util.OptionalObjectProperty; @@ -57,8 +65,40 @@ public class StateManager { private final EasyBinding tasksProgress = EasyBind.reduce(backgroundTasks, tasks -> tasks.filter(Task::isRunning).mapToDouble(Task::getProgress).average().orElse(1)); private final ObservableMap dialogWindowStates = FXCollections.observableHashMap(); + private final ObservableList visiblePanes = FXCollections.observableArrayList(); + + private final BooleanProperty groupsPaneVisible = new SimpleBooleanProperty(); + private final BooleanProperty openOfficePaneVisible = new SimpleBooleanProperty(); + private final BooleanProperty webSearchPaneVisible = new SimpleBooleanProperty(); + public StateManager() { activeGroups.bind(Bindings.valueAt(selectedGroups, activeDatabase.orElse(null))); + + visiblePanes.addListener((ListChangeListener) change -> { + while (change.next()) { + if (change.wasAdded()) { + SidePaneType added = change.getAddedSubList().get(0); + sidePaneComponentVisibleProperty(added).set(true); + Globals.prefs.getSidePanePreferences().visiblePanes().add(added); + } else if (change.wasRemoved()) { + SidePaneType removed = change.getRemoved().get(0); + sidePaneComponentVisibleProperty(removed).set(false); + Globals.prefs.getSidePanePreferences().visiblePanes().remove(removed); + } + } + }); + } + + public BooleanProperty sidePaneComponentVisibleProperty(SidePaneType component) { + return switch (component) { + case WEB_SEARCH -> webSearchPaneVisible; + case GROUPS -> groupsPaneVisible; + case OPEN_OFFICE -> openOfficePaneVisible; + }; + } + + public ObservableList getVisibleSidePaneComponents() { + return visiblePanes; } public CustomLocalDragboard getLocalDragboard() { diff --git a/src/main/java/org/jabref/gui/sidepane/SidePane.java b/src/main/java/org/jabref/gui/sidepane/SidePane.java index ddbe4c88449..72a2c502fac 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePane.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePane.java @@ -14,7 +14,11 @@ import org.jabref.gui.util.TaskExecutor; import org.jabref.preferences.PreferencesService; +import com.tobiasdiez.easybind.EasyBind; + import static org.jabref.gui.sidepane.SidePaneType.GROUPS; +import static org.jabref.gui.sidepane.SidePaneType.OPEN_OFFICE; +import static org.jabref.gui.sidepane.SidePaneType.WEB_SEARCH; public class SidePane extends VBox { // Don't use this map directly to lookup sidePaneViews, instead use getSidePaneView() for lazy loading @@ -40,12 +44,24 @@ public SidePane(PreferencesService preferencesService, this.stateManager = stateManager; this.undoManager = undoManager; this.sidePaneContentFactory = new SidePaneContentFactory(preferencesService, taskExecutor, dialogService, stateManager, undoManager); - this.viewModel = new SidePaneViewModel(preferencesService); + this.viewModel = new SidePaneViewModel(preferencesService, stateManager); + + EasyBind.subscribe(stateManager.sidePaneComponentVisibleProperty(GROUPS), isShow -> showOrHidePane(GROUPS, isShow)); + EasyBind.subscribe(stateManager.sidePaneComponentVisibleProperty(WEB_SEARCH), isShow -> showOrHidePane(WEB_SEARCH, isShow)); + EasyBind.subscribe(stateManager.sidePaneComponentVisibleProperty(OPEN_OFFICE), isShow -> showOrHidePane(OPEN_OFFICE, isShow)); preferencesService.getSidePanePreferences().visiblePanes().forEach(this::show); updateView(); } + private void showOrHidePane(SidePaneType pane, boolean isShow) { + if (isShow) { + show(pane); + } else { + hide(pane); + } + } + private SidePaneComponent getSidePaneComponent(SidePaneType pane) { SidePaneComponent sidePaneComponent = sidePaneComponentLookup.get(pane); if (sidePaneComponent == null) { @@ -110,11 +126,7 @@ private void updateView() { } public BooleanProperty paneVisibleProperty(SidePaneType pane) { - return switch (pane) { - case GROUPS -> viewModel.groupsPaneVisibleProperty(); - case WEB_SEARCH -> viewModel.webSearchPaneVisibleProperty(); - case OPEN_OFFICE -> viewModel.openOfficePaneVisibleProperty(); - }; + return stateManager.sidePaneComponentVisibleProperty(pane); } public ToggleCommand getToggleCommandFor(SidePaneType sidePane) { diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java index 807d897c4e1..24c85e81434 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java @@ -8,30 +8,19 @@ import java.util.Map; import java.util.stream.IntStream; -import javafx.beans.binding.Bindings; -import javafx.beans.property.BooleanProperty; -import javafx.beans.property.SimpleBooleanProperty; -import javafx.collections.FXCollections; import javafx.collections.ObservableList; import org.jabref.gui.AbstractViewModel; +import org.jabref.gui.StateManager; import org.jabref.preferences.PreferencesService; public class SidePaneViewModel extends AbstractViewModel { private final PreferencesService preferencesService; - // TODO('Use preferencesService.getSidePanePreferences().visiblePanes() as the single source of truth') - private final ObservableList visiblePanes = FXCollections.observableArrayList(); + private final StateManager stateManager; - private final BooleanProperty groupsPaneVisible = new SimpleBooleanProperty(); - private final BooleanProperty openOfficePaneVisible = new SimpleBooleanProperty(); - private final BooleanProperty webSearchPaneVisible = new SimpleBooleanProperty(); - - public SidePaneViewModel(PreferencesService preferencesService) { + public SidePaneViewModel(PreferencesService preferencesService, StateManager stateManager) { this.preferencesService = preferencesService; - - groupsPaneVisible.bind(Bindings.createBooleanBinding(() -> visiblePanes.contains(SidePaneType.GROUPS), visiblePanes)); - openOfficePaneVisible.bind(Bindings.createBooleanBinding(() -> visiblePanes.contains(SidePaneType.OPEN_OFFICE), visiblePanes)); - webSearchPaneVisible.bind(Bindings.createBooleanBinding(() -> visiblePanes.contains(SidePaneType.WEB_SEARCH), visiblePanes)); + this.stateManager = stateManager; } /** @@ -40,23 +29,23 @@ public SidePaneViewModel(PreferencesService preferencesService) { */ private void updatePreferredPositions() { Map preferredPositions = new HashMap<>(preferencesService.getSidePanePreferences().getPreferredPositions()); - IntStream.range(0, visiblePanes.size()).forEach(i -> preferredPositions.put(visiblePanes.get(i), i)); + IntStream.range(0, getVisiblePanes().size()).forEach(i -> preferredPositions.put(getVisiblePanes().get(i), i)); preferencesService.getSidePanePreferences().setPreferredPositions(preferredPositions); } public ObservableList getVisiblePanes() { - return visiblePanes; + return stateManager.getVisibleSidePaneComponents(); } /** * @return True if pane is visible, and it can still move up therefore we should update the view */ public boolean moveUp(SidePaneType pane) { - if (visiblePanes.contains(pane)) { - int currentPosition = visiblePanes.indexOf(pane); + if (getVisiblePanes().contains(pane)) { + int currentPosition = getVisiblePanes().indexOf(pane); if (currentPosition > 0) { int newPosition = currentPosition - 1; - swap(visiblePanes, currentPosition, newPosition); + swap(getVisiblePanes(), currentPosition, newPosition); updatePreferredPositions(); return true; } @@ -68,11 +57,11 @@ public boolean moveUp(SidePaneType pane) { * @return True if pane is visible, and it can still move down therefore we should update the view */ public boolean moveDown(SidePaneType pane) { - if (visiblePanes.contains(pane)) { - int currentPosition = visiblePanes.indexOf(pane); - if (currentPosition < (visiblePanes.size() - 1)) { + if (getVisiblePanes().contains(pane)) { + int currentPosition = getVisiblePanes().indexOf(pane); + if (currentPosition < (getVisiblePanes().size() - 1)) { int newPosition = currentPosition + 1; - swap(visiblePanes, currentPosition, newPosition); + swap(getVisiblePanes(), currentPosition, newPosition); updatePreferredPositions(); return true; } @@ -84,10 +73,9 @@ public boolean moveDown(SidePaneType pane) { * @return True if pane is not already shown which means the view needs to be updated */ public boolean show(SidePaneType pane) { - if (!visiblePanes.contains(pane)) { - visiblePanes.add(pane); - preferencesService.getSidePanePreferences().visiblePanes().add(pane); - visiblePanes.sorted(new PreferredIndexSort(preferencesService)); + if (!getVisiblePanes().contains(pane)) { + getVisiblePanes().add(pane); + getVisiblePanes().sort(new PreferredIndexSort(preferencesService)); return true; } return false; @@ -97,29 +85,16 @@ public boolean show(SidePaneType pane) { * @return True if pane is visible which means the view needs to be updated */ public boolean hide(SidePaneType pane) { - if (visiblePanes.contains(pane)) { - visiblePanes.remove(pane); - preferencesService.getSidePanePreferences().visiblePanes().remove(pane); + if (getVisiblePanes().contains(pane)) { + getVisiblePanes().remove(pane); return true; } else { return false; } } - public BooleanProperty groupsPaneVisibleProperty() { - return groupsPaneVisible; - } - - public BooleanProperty openOfficePaneVisibleProperty() { - return openOfficePaneVisible; - } - - public BooleanProperty webSearchPaneVisibleProperty() { - return webSearchPaneVisible; - } - public boolean isPaneVisible(SidePaneType pane) { - return visiblePanes.contains(pane); + return getVisiblePanes().contains(pane); } /** From b0cae97ffb2e067fee1cabece0a9033ec9fdde69 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sun, 7 Nov 2021 00:29:19 +0100 Subject: [PATCH 24/36] Remove comment --- src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java index 24c85e81434..8ae8879a3d0 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java @@ -96,10 +96,7 @@ public boolean hide(SidePaneType pane) { public boolean isPaneVisible(SidePaneType pane) { return getVisiblePanes().contains(pane); } - - /** - * This implementation is inefficient because of some JavaFX limitations, we only advice to use it on small lists - */ + private void swap(ObservableList observableList, int i, int j) { List placeholder = new ArrayList<>(observableList); Collections.swap(placeholder, i, j); From 85308f760d78f9c92cc7da0055098a6b044e19fd Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sun, 7 Nov 2021 00:44:32 +0100 Subject: [PATCH 25/36] Use better names --- src/main/java/org/jabref/gui/StateManager.java | 18 +++++++----------- .../java/org/jabref/gui/sidepane/SidePane.java | 8 ++++---- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/jabref/gui/StateManager.java b/src/main/java/org/jabref/gui/StateManager.java index 40bba82a6b4..cd945c5d0c9 100644 --- a/src/main/java/org/jabref/gui/StateManager.java +++ b/src/main/java/org/jabref/gui/StateManager.java @@ -1,14 +1,10 @@ package org.jabref.gui; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; -import java.util.stream.IntStream; -import javafx.beans.InvalidationListener; import javafx.beans.Observable; import javafx.beans.binding.Bindings; import javafx.beans.property.BooleanProperty; @@ -77,19 +73,19 @@ public StateManager() { visiblePanes.addListener((ListChangeListener) change -> { while (change.next()) { if (change.wasAdded()) { - SidePaneType added = change.getAddedSubList().get(0); - sidePaneComponentVisibleProperty(added).set(true); - Globals.prefs.getSidePanePreferences().visiblePanes().add(added); + SidePaneType visibleComponent = change.getAddedSubList().get(0); + sidePaneComponentVisiblePropertyFor(visibleComponent).set(true); + Globals.prefs.getSidePanePreferences().visiblePanes().add(visibleComponent); } else if (change.wasRemoved()) { - SidePaneType removed = change.getRemoved().get(0); - sidePaneComponentVisibleProperty(removed).set(false); - Globals.prefs.getSidePanePreferences().visiblePanes().remove(removed); + SidePaneType hiddenComponent = change.getRemoved().get(0); + sidePaneComponentVisiblePropertyFor(hiddenComponent).set(false); + Globals.prefs.getSidePanePreferences().visiblePanes().remove(hiddenComponent); } } }); } - public BooleanProperty sidePaneComponentVisibleProperty(SidePaneType component) { + public BooleanProperty sidePaneComponentVisiblePropertyFor(SidePaneType component) { return switch (component) { case WEB_SEARCH -> webSearchPaneVisible; case GROUPS -> groupsPaneVisible; diff --git a/src/main/java/org/jabref/gui/sidepane/SidePane.java b/src/main/java/org/jabref/gui/sidepane/SidePane.java index 72a2c502fac..b31a27a2776 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePane.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePane.java @@ -46,9 +46,9 @@ public SidePane(PreferencesService preferencesService, this.sidePaneContentFactory = new SidePaneContentFactory(preferencesService, taskExecutor, dialogService, stateManager, undoManager); this.viewModel = new SidePaneViewModel(preferencesService, stateManager); - EasyBind.subscribe(stateManager.sidePaneComponentVisibleProperty(GROUPS), isShow -> showOrHidePane(GROUPS, isShow)); - EasyBind.subscribe(stateManager.sidePaneComponentVisibleProperty(WEB_SEARCH), isShow -> showOrHidePane(WEB_SEARCH, isShow)); - EasyBind.subscribe(stateManager.sidePaneComponentVisibleProperty(OPEN_OFFICE), isShow -> showOrHidePane(OPEN_OFFICE, isShow)); + EasyBind.subscribe(stateManager.sidePaneComponentVisiblePropertyFor(GROUPS), isShow -> showOrHidePane(GROUPS, isShow)); + EasyBind.subscribe(stateManager.sidePaneComponentVisiblePropertyFor(WEB_SEARCH), isShow -> showOrHidePane(WEB_SEARCH, isShow)); + EasyBind.subscribe(stateManager.sidePaneComponentVisiblePropertyFor(OPEN_OFFICE), isShow -> showOrHidePane(OPEN_OFFICE, isShow)); preferencesService.getSidePanePreferences().visiblePanes().forEach(this::show); updateView(); @@ -126,7 +126,7 @@ private void updateView() { } public BooleanProperty paneVisibleProperty(SidePaneType pane) { - return stateManager.sidePaneComponentVisibleProperty(pane); + return stateManager.sidePaneComponentVisiblePropertyFor(pane); } public ToggleCommand getToggleCommandFor(SidePaneType sidePane) { From e44ae48b7c7b815a387bf28a3dabc298eabdf90d Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sun, 7 Nov 2021 00:52:24 +0100 Subject: [PATCH 26/36] Fix checkstyle --- src/main/java/org/jabref/gui/JabRefFrame.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index 05aa4806437..d20c7cae135 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -25,7 +25,6 @@ import javafx.scene.control.Button; import javafx.scene.control.ButtonBar; import javafx.scene.control.ButtonType; -import javafx.scene.control.CheckMenuItem; import javafx.scene.control.ContextMenu; import javafx.scene.control.Menu; import javafx.scene.control.MenuBar; From 26163351793a3c0250b2831a1d0e95dd47473b77 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Mon, 8 Nov 2021 04:47:52 +0100 Subject: [PATCH 27/36] Move close/toggle pane actions to separate files --- .../jabref/gui/sidepane/ClosePaneAction.java | 19 ++++++++ .../org/jabref/gui/sidepane/SidePane.java | 46 ++----------------- .../jabref/gui/sidepane/TogglePaneAction.java | 20 ++++++++ 3 files changed, 43 insertions(+), 42 deletions(-) create mode 100644 src/main/java/org/jabref/gui/sidepane/ClosePaneAction.java create mode 100644 src/main/java/org/jabref/gui/sidepane/TogglePaneAction.java diff --git a/src/main/java/org/jabref/gui/sidepane/ClosePaneAction.java b/src/main/java/org/jabref/gui/sidepane/ClosePaneAction.java new file mode 100644 index 00000000000..c0b56311a43 --- /dev/null +++ b/src/main/java/org/jabref/gui/sidepane/ClosePaneAction.java @@ -0,0 +1,19 @@ +package org.jabref.gui.sidepane; + +import org.jabref.gui.StateManager; +import org.jabref.gui.actions.SimpleCommand; + +public class ClosePaneAction extends SimpleCommand { + private final StateManager stateManager; + private final SidePaneType toClosePane; + + public ClosePaneAction(StateManager stateManager, SidePaneType toClosePane) { + this.stateManager = stateManager; + this.toClosePane = toClosePane; + } + + @Override + public void execute() { + stateManager.sidePaneComponentVisiblePropertyFor(toClosePane).set(false); + } +} diff --git a/src/main/java/org/jabref/gui/sidepane/SidePane.java b/src/main/java/org/jabref/gui/sidepane/SidePane.java index b31a27a2776..ed6f8661bf7 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePane.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePane.java @@ -66,8 +66,8 @@ private SidePaneComponent getSidePaneComponent(SidePaneType pane) { SidePaneComponent sidePaneComponent = sidePaneComponentLookup.get(pane); if (sidePaneComponent == null) { sidePaneComponent = switch (pane) { - case GROUPS -> new GroupsSidePaneComponent(new ClosePaneAction(pane), new MoveUpAction(pane), new MoveDownAction(pane), sidePaneContentFactory, preferencesService, dialogService); - case WEB_SEARCH, OPEN_OFFICE -> new SidePaneComponent(pane, new ClosePaneAction(pane), new MoveUpAction(pane), new MoveDownAction(pane), sidePaneContentFactory); + case GROUPS -> new GroupsSidePaneComponent(new ClosePaneAction(stateManager, pane), new MoveUpAction(pane), new MoveDownAction(pane), sidePaneContentFactory, preferencesService, dialogService); + case WEB_SEARCH, OPEN_OFFICE -> new SidePaneComponent(pane, new ClosePaneAction(stateManager, pane), new MoveUpAction(pane), new MoveDownAction(pane), sidePaneContentFactory); }; sidePaneComponentLookup.put(pane, sidePaneComponent); } @@ -109,17 +109,6 @@ private void moveDown(SidePaneType pane) { } } - /** - * If the given component is visible it will be hidden and the other way around. - */ - private void toggle(SidePaneType pane) { - if (viewModel.isPaneVisible(pane)) { - hide(pane); - } else { - show(pane); - } - } - private void updateView() { showVisiblePanes(); setVisible(!viewModel.getVisiblePanes().isEmpty()); @@ -129,21 +118,8 @@ public BooleanProperty paneVisibleProperty(SidePaneType pane) { return stateManager.sidePaneComponentVisiblePropertyFor(pane); } - public ToggleCommand getToggleCommandFor(SidePaneType sidePane) { - return new ToggleCommand(sidePane); - } - - private class ClosePaneAction extends SimpleCommand { - private final SidePaneType toClosePane; - - public ClosePaneAction(SidePaneType toClosePane) { - this.toClosePane = toClosePane; - } - - @Override - public void execute() { - hide(toClosePane); - } + public SimpleCommand getToggleCommandFor(SidePaneType sidePane) { + return new TogglePaneAction(stateManager, sidePane); } private class MoveUpAction extends SimpleCommand { @@ -171,18 +147,4 @@ public void execute() { moveDown(toMoveDownPane); } } - - public class ToggleCommand extends SimpleCommand { - - private final SidePaneType pane; - - public ToggleCommand(SidePaneType pane) { - this.pane = pane; - } - - @Override - public void execute() { - toggle(pane); - } - } } diff --git a/src/main/java/org/jabref/gui/sidepane/TogglePaneAction.java b/src/main/java/org/jabref/gui/sidepane/TogglePaneAction.java new file mode 100644 index 00000000000..fccf961cab4 --- /dev/null +++ b/src/main/java/org/jabref/gui/sidepane/TogglePaneAction.java @@ -0,0 +1,20 @@ +package org.jabref.gui.sidepane; + +import org.jabref.gui.StateManager; +import org.jabref.gui.actions.SimpleCommand; + +public class TogglePaneAction extends SimpleCommand { + private final StateManager stateManager; + private final SidePaneType pane; + + public TogglePaneAction(StateManager stateManager, SidePaneType pane) { + this.stateManager = stateManager; + this.pane = pane; + } + + @Override + public void execute() { + boolean isVisible = stateManager.sidePaneComponentVisiblePropertyFor(pane).get(); + stateManager.sidePaneComponentVisiblePropertyFor(pane).set(!isVisible); + } +} From ab49960ffbb2adb2490d12ba83d0e38f68e57417 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Sun, 14 Nov 2021 17:48:26 +0100 Subject: [PATCH 28/36] Move sidepane binding logic to SidePaneViewModel --- .../java/org/jabref/gui/StateManager.java | 15 ------------ .../gui/sidepane/SidePaneViewModel.java | 23 ++++++++++++++++++- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/jabref/gui/StateManager.java b/src/main/java/org/jabref/gui/StateManager.java index cd945c5d0c9..fa14f4da0ef 100644 --- a/src/main/java/org/jabref/gui/StateManager.java +++ b/src/main/java/org/jabref/gui/StateManager.java @@ -14,7 +14,6 @@ import javafx.beans.property.SimpleBooleanProperty; import javafx.beans.property.SimpleIntegerProperty; import javafx.collections.FXCollections; -import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; import javafx.collections.ObservableMap; import javafx.concurrent.Task; @@ -69,20 +68,6 @@ public class StateManager { public StateManager() { activeGroups.bind(Bindings.valueAt(selectedGroups, activeDatabase.orElse(null))); - - visiblePanes.addListener((ListChangeListener) change -> { - while (change.next()) { - if (change.wasAdded()) { - SidePaneType visibleComponent = change.getAddedSubList().get(0); - sidePaneComponentVisiblePropertyFor(visibleComponent).set(true); - Globals.prefs.getSidePanePreferences().visiblePanes().add(visibleComponent); - } else if (change.wasRemoved()) { - SidePaneType hiddenComponent = change.getRemoved().get(0); - sidePaneComponentVisiblePropertyFor(hiddenComponent).set(false); - Globals.prefs.getSidePanePreferences().visiblePanes().remove(hiddenComponent); - } - } - }); } public BooleanProperty sidePaneComponentVisiblePropertyFor(SidePaneType component) { diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java index 8ae8879a3d0..2499173ac1b 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java @@ -8,6 +8,7 @@ import java.util.Map; import java.util.stream.IntStream; +import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; import org.jabref.gui.AbstractViewModel; @@ -21,6 +22,26 @@ public class SidePaneViewModel extends AbstractViewModel { public SidePaneViewModel(PreferencesService preferencesService, StateManager stateManager) { this.preferencesService = preferencesService; this.stateManager = stateManager; + + getVisiblePanes().addListener((ListChangeListener) change -> { + while (change.next()) { + if (change.wasAdded()) { + onPaneAdded(change.getAddedSubList().get(0)); + } else if (change.wasRemoved()) { + onPaneRemoved(change.getRemoved().get(0)); + } + } + }); + } + + private void onPaneAdded(SidePaneType pane) { + stateManager.sidePaneComponentVisiblePropertyFor(pane).set(true); + preferencesService.getSidePanePreferences().visiblePanes().add(pane); + } + + private void onPaneRemoved(SidePaneType pane) { + stateManager.sidePaneComponentVisiblePropertyFor(pane).set(false); + preferencesService.getSidePanePreferences().visiblePanes().remove(pane); } /** @@ -96,7 +117,7 @@ public boolean hide(SidePaneType pane) { public boolean isPaneVisible(SidePaneType pane) { return getVisiblePanes().contains(pane); } - + private void swap(ObservableList observableList, int i, int j) { List placeholder = new ArrayList<>(observableList); Collections.swap(placeholder, i, j); From e573811a7643254435b7d7590f3247209aa653b2 Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage Date: Sun, 5 Dec 2021 21:48:57 +0100 Subject: [PATCH 29/36] Removed unnecessary properties in stateManager, refactored to mvvm pattern --- src/main/java/org/jabref/gui/JabRefFrame.java | 6 +- .../java/org/jabref/gui/StateManager.java | 19 +- .../org/jabref/gui/actions/ActionFactory.java | 7 +- .../jabref/gui/sidepane/ClosePaneAction.java | 19 -- .../gui/sidepane/GroupsSidePaneComponent.java | 7 +- .../org/jabref/gui/sidepane/SidePane.java | 134 ++---------- .../gui/sidepane/SidePaneViewModel.java | 191 ++++++++++++------ .../jabref/gui/sidepane/TogglePaneAction.java | 14 +- 8 files changed, 177 insertions(+), 220 deletions(-) delete mode 100644 src/main/java/org/jabref/gui/sidepane/ClosePaneAction.java diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index 6150f27a741..04c59948a21 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -839,9 +839,9 @@ private MenuBar createMenu() { SidePaneType groupsPane = SidePaneType.GROUPS; SidePaneType openOfficePane = SidePaneType.OPEN_OFFICE; view.getItems().addAll( - factory.createCheckMenuItem(webSearchPane.getToggleAction(), sidePane.getToggleCommandFor(webSearchPane), sidePane.paneVisibleProperty(webSearchPane)), - factory.createCheckMenuItem(groupsPane.getToggleAction(), sidePane.getToggleCommandFor(groupsPane), sidePane.paneVisibleProperty(groupsPane)), - factory.createCheckMenuItem(openOfficePane.getToggleAction(), sidePane.getToggleCommandFor(openOfficePane), sidePane.paneVisibleProperty(openOfficePane)), + factory.createCheckMenuItem(webSearchPane.getToggleAction(), sidePane.getToggleCommandFor(webSearchPane), sidePane.paneVisibleBinding(webSearchPane)), + factory.createCheckMenuItem(groupsPane.getToggleAction(), sidePane.getToggleCommandFor(groupsPane), sidePane.paneVisibleBinding(groupsPane)), + factory.createCheckMenuItem(openOfficePane.getToggleAction(), sidePane.getToggleCommandFor(openOfficePane), sidePane.paneVisibleBinding(openOfficePane)), new SeparatorMenuItem(), diff --git a/src/main/java/org/jabref/gui/StateManager.java b/src/main/java/org/jabref/gui/StateManager.java index fa14f4da0ef..386e08779e9 100644 --- a/src/main/java/org/jabref/gui/StateManager.java +++ b/src/main/java/org/jabref/gui/StateManager.java @@ -7,11 +7,9 @@ import javafx.beans.Observable; import javafx.beans.binding.Bindings; -import javafx.beans.property.BooleanProperty; import javafx.beans.property.IntegerProperty; import javafx.beans.property.ReadOnlyListProperty; import javafx.beans.property.ReadOnlyListWrapper; -import javafx.beans.property.SimpleBooleanProperty; import javafx.beans.property.SimpleIntegerProperty; import javafx.collections.FXCollections; import javafx.collections.ObservableList; @@ -59,27 +57,14 @@ public class StateManager { private final EasyBinding anyTaskRunning = EasyBind.reduce(backgroundTasks, tasks -> tasks.anyMatch(Task::isRunning)); private final EasyBinding tasksProgress = EasyBind.reduce(backgroundTasks, tasks -> tasks.filter(Task::isRunning).mapToDouble(Task::getProgress).average().orElse(1)); private final ObservableMap dialogWindowStates = FXCollections.observableHashMap(); - - private final ObservableList visiblePanes = FXCollections.observableArrayList(); - - private final BooleanProperty groupsPaneVisible = new SimpleBooleanProperty(); - private final BooleanProperty openOfficePaneVisible = new SimpleBooleanProperty(); - private final BooleanProperty webSearchPaneVisible = new SimpleBooleanProperty(); + private final ObservableList visibleSidePanes = FXCollections.observableArrayList(); public StateManager() { activeGroups.bind(Bindings.valueAt(selectedGroups, activeDatabase.orElse(null))); } - public BooleanProperty sidePaneComponentVisiblePropertyFor(SidePaneType component) { - return switch (component) { - case WEB_SEARCH -> webSearchPaneVisible; - case GROUPS -> groupsPaneVisible; - case OPEN_OFFICE -> openOfficePaneVisible; - }; - } - public ObservableList getVisibleSidePaneComponents() { - return visiblePanes; + return visibleSidePanes; } public CustomLocalDragboard getLocalDragboard() { diff --git a/src/main/java/org/jabref/gui/actions/ActionFactory.java b/src/main/java/org/jabref/gui/actions/ActionFactory.java index 9be7a2c63d0..7112ffc8f12 100644 --- a/src/main/java/org/jabref/gui/actions/ActionFactory.java +++ b/src/main/java/org/jabref/gui/actions/ActionFactory.java @@ -5,7 +5,7 @@ import java.lang.reflect.Method; import java.util.Objects; -import javafx.beans.property.BooleanProperty; +import javafx.beans.binding.BooleanExpression; import javafx.scene.control.Button; import javafx.scene.control.ButtonBase; import javafx.scene.control.CheckMenuItem; @@ -114,10 +114,9 @@ public CheckMenuItem createCheckMenuItem(Action action, Command command, boolean return checkMenuItem; } - public CheckMenuItem createCheckMenuItem(Action action, Command command, BooleanProperty selectedProperty) { + public CheckMenuItem createCheckMenuItem(Action action, Command command, BooleanExpression selectedBinding) { CheckMenuItem checkMenuItem = ActionUtils.createCheckMenuItem(new JabRefAction(action, command, keyBindingRepository, Sources.FromMenu)); - checkMenuItem.setSelected(selectedProperty.getValue()); - EasyBind.subscribe(selectedProperty, checkMenuItem::setSelected); + EasyBind.subscribe(selectedBinding, checkMenuItem::setSelected); setGraphic(checkMenuItem, action); return checkMenuItem; diff --git a/src/main/java/org/jabref/gui/sidepane/ClosePaneAction.java b/src/main/java/org/jabref/gui/sidepane/ClosePaneAction.java deleted file mode 100644 index c0b56311a43..00000000000 --- a/src/main/java/org/jabref/gui/sidepane/ClosePaneAction.java +++ /dev/null @@ -1,19 +0,0 @@ -package org.jabref.gui.sidepane; - -import org.jabref.gui.StateManager; -import org.jabref.gui.actions.SimpleCommand; - -public class ClosePaneAction extends SimpleCommand { - private final StateManager stateManager; - private final SidePaneType toClosePane; - - public ClosePaneAction(StateManager stateManager, SidePaneType toClosePane) { - this.stateManager = stateManager; - this.toClosePane = toClosePane; - } - - @Override - public void execute() { - stateManager.sidePaneComponentVisiblePropertyFor(toClosePane).set(false); - } -} diff --git a/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneComponent.java b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneComponent.java index 695916854ad..bba07caf38a 100644 --- a/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneComponent.java +++ b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneComponent.java @@ -15,7 +15,12 @@ public class GroupsSidePaneComponent extends SidePaneComponent { private final DialogService dialogService; private final Button intersectionUnionToggle = IconTheme.JabRefIcons.GROUP_INTERSECTION.asButton(); - public GroupsSidePaneComponent(SimpleCommand closeCommand, SimpleCommand moveUpCommand, SimpleCommand moveDownCommand, SidePaneContentFactory contentFactory, PreferencesService preferences, DialogService dialogService) { + public GroupsSidePaneComponent(SimpleCommand closeCommand, + SimpleCommand moveUpCommand, + SimpleCommand moveDownCommand, + SidePaneContentFactory contentFactory, + PreferencesService preferences, + DialogService dialogService) { super(SidePaneType.GROUPS, closeCommand, moveUpCommand, moveDownCommand, contentFactory); this.preferences = preferences; this.dialogService = dialogService; diff --git a/src/main/java/org/jabref/gui/sidepane/SidePane.java b/src/main/java/org/jabref/gui/sidepane/SidePane.java index ed6f8661bf7..6e9476abcb7 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePane.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePane.java @@ -5,7 +5,9 @@ import javax.swing.undo.UndoManager; -import javafx.beans.property.BooleanProperty; +import javafx.beans.binding.Bindings; +import javafx.beans.binding.BooleanBinding; +import javafx.collections.ListChangeListener; import javafx.scene.layout.VBox; import org.jabref.gui.DialogService; @@ -14,137 +16,45 @@ import org.jabref.gui.util.TaskExecutor; import org.jabref.preferences.PreferencesService; -import com.tobiasdiez.easybind.EasyBind; - -import static org.jabref.gui.sidepane.SidePaneType.GROUPS; -import static org.jabref.gui.sidepane.SidePaneType.OPEN_OFFICE; -import static org.jabref.gui.sidepane.SidePaneType.WEB_SEARCH; - public class SidePane extends VBox { - // Don't use this map directly to lookup sidePaneViews, instead use getSidePaneView() for lazy loading - private final Map sidePaneComponentLookup = new HashMap<>(); private final SidePaneViewModel viewModel; - private final PreferencesService preferencesService; - private final TaskExecutor taskExecutor; - private final DialogService dialogService; private final StateManager stateManager; - private final UndoManager undoManager; - private final SidePaneContentFactory sidePaneContentFactory; + // These bindings need to be stored, otherwise they are garbage collected + @SuppressWarnings("MismatchedQueryAndUpdateOfCollection") + private final Map visibleBindings = new HashMap<>(); public SidePane(PreferencesService preferencesService, TaskExecutor taskExecutor, DialogService dialogService, StateManager stateManager, UndoManager undoManager) { - this.preferencesService = preferencesService; - this.taskExecutor = taskExecutor; - this.dialogService = dialogService; this.stateManager = stateManager; - this.undoManager = undoManager; - this.sidePaneContentFactory = new SidePaneContentFactory(preferencesService, taskExecutor, dialogService, stateManager, undoManager); - this.viewModel = new SidePaneViewModel(preferencesService, stateManager); - - EasyBind.subscribe(stateManager.sidePaneComponentVisiblePropertyFor(GROUPS), isShow -> showOrHidePane(GROUPS, isShow)); - EasyBind.subscribe(stateManager.sidePaneComponentVisiblePropertyFor(WEB_SEARCH), isShow -> showOrHidePane(WEB_SEARCH, isShow)); - EasyBind.subscribe(stateManager.sidePaneComponentVisiblePropertyFor(OPEN_OFFICE), isShow -> showOrHidePane(OPEN_OFFICE, isShow)); + this.preferencesService = preferencesService; + this.viewModel = new SidePaneViewModel(preferencesService, stateManager, taskExecutor, dialogService, undoManager); - preferencesService.getSidePanePreferences().visiblePanes().forEach(this::show); + stateManager.getVisibleSidePaneComponents().addListener((ListChangeListener) c -> updateView()); updateView(); } - private void showOrHidePane(SidePaneType pane, boolean isShow) { - if (isShow) { - show(pane); - } else { - hide(pane); - } - } - - private SidePaneComponent getSidePaneComponent(SidePaneType pane) { - SidePaneComponent sidePaneComponent = sidePaneComponentLookup.get(pane); - if (sidePaneComponent == null) { - sidePaneComponent = switch (pane) { - case GROUPS -> new GroupsSidePaneComponent(new ClosePaneAction(stateManager, pane), new MoveUpAction(pane), new MoveDownAction(pane), sidePaneContentFactory, preferencesService, dialogService); - case WEB_SEARCH, OPEN_OFFICE -> new SidePaneComponent(pane, new ClosePaneAction(stateManager, pane), new MoveUpAction(pane), new MoveDownAction(pane), sidePaneContentFactory); - }; - sidePaneComponentLookup.put(pane, sidePaneComponent); - } - return sidePaneComponent; - } - - private void showVisiblePanes() { + private void updateView() { getChildren().clear(); - viewModel.getVisiblePanes().forEach(type -> { - SidePaneComponent view = getSidePaneComponent(type); - getChildren().add(view); - }); - } - - private void show(SidePaneType pane) { - if (viewModel.show(pane)) { - updateView(); - if (pane == GROUPS) { - ((GroupsSidePaneComponent) getSidePaneComponent(pane)).afterOpening(); - } - } - } - - private void hide(SidePaneType pane) { - if (viewModel.hide(pane)) { - updateView(); - } - } + for (SidePaneType type : stateManager.getVisibleSidePaneComponents()) { + SidePaneComponent view = viewModel.getSidePaneComponent(type); + getChildren().add(view); + } + } - private void moveUp(SidePaneType pane) { - if (viewModel.moveUp(pane)) { - updateView(); - } - } - - private void moveDown(SidePaneType pane) { - if (viewModel.moveDown(pane)) { - updateView(); - } - } - - private void updateView() { - showVisiblePanes(); - setVisible(!viewModel.getVisiblePanes().isEmpty()); - } - - public BooleanProperty paneVisibleProperty(SidePaneType pane) { - return stateManager.sidePaneComponentVisiblePropertyFor(pane); + public BooleanBinding paneVisibleBinding(SidePaneType pane) { + BooleanBinding visibility = Bindings.createBooleanBinding( + () -> stateManager.getVisibleSidePaneComponents().contains(pane), + stateManager.getVisibleSidePaneComponents()); + visibleBindings.put(pane, visibility); + return visibility; } public SimpleCommand getToggleCommandFor(SidePaneType sidePane) { - return new TogglePaneAction(stateManager, sidePane); - } - - private class MoveUpAction extends SimpleCommand { - private final SidePaneType toMoveUpPane; - - public MoveUpAction(SidePaneType toMoveUpPane) { - this.toMoveUpPane = toMoveUpPane; - } - - @Override - public void execute() { - moveUp(toMoveUpPane); - } - } - - private class MoveDownAction extends SimpleCommand { - private final SidePaneType toMoveDownPane; - - public MoveDownAction(SidePaneType toMoveDownPane) { - this.toMoveDownPane = toMoveDownPane; - } - - @Override - public void execute() { - moveDown(toMoveDownPane); - } + return new TogglePaneAction(stateManager, sidePane, preferencesService.getSidePanePreferences()); } } diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java index 2499173ac1b..979289e149d 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java @@ -8,114 +8,144 @@ import java.util.Map; import java.util.stream.IntStream; +import javax.swing.undo.UndoManager; + import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; import org.jabref.gui.AbstractViewModel; +import org.jabref.gui.DialogService; import org.jabref.gui.StateManager; +import org.jabref.gui.actions.SimpleCommand; +import org.jabref.gui.util.TaskExecutor; import org.jabref.preferences.PreferencesService; +import org.jabref.preferences.SidePanePreferences; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.jabref.gui.sidepane.SidePaneType.GROUPS; public class SidePaneViewModel extends AbstractViewModel { + private static final Logger LOGGER = LoggerFactory.getLogger(SidePaneViewModel.class); + + private final Map sidePaneComponentLookup = new HashMap<>(); + private final PreferencesService preferencesService; private final StateManager stateManager; - - public SidePaneViewModel(PreferencesService preferencesService, StateManager stateManager) { + private final SidePaneContentFactory sidePaneContentFactory; + private final DialogService dialogService; + + public SidePaneViewModel(PreferencesService preferencesService, + StateManager stateManager, + TaskExecutor taskExecutor, + DialogService dialogService, + UndoManager undoManager) { this.preferencesService = preferencesService; this.stateManager = stateManager; + this.dialogService = dialogService; + this.sidePaneContentFactory = new SidePaneContentFactory(preferencesService, taskExecutor, dialogService, stateManager, undoManager); - getVisiblePanes().addListener((ListChangeListener) change -> { + preferencesService.getSidePanePreferences().visiblePanes().forEach(this::show); + getPanes().addListener((ListChangeListener) change -> { while (change.next()) { if (change.wasAdded()) { - onPaneAdded(change.getAddedSubList().get(0)); + preferencesService.getSidePanePreferences().visiblePanes().add(change.getAddedSubList().get(0)); } else if (change.wasRemoved()) { - onPaneRemoved(change.getRemoved().get(0)); + preferencesService.getSidePanePreferences().visiblePanes().remove(change.getRemoved().get(0)); } } }); } - private void onPaneAdded(SidePaneType pane) { - stateManager.sidePaneComponentVisiblePropertyFor(pane).set(true); - preferencesService.getSidePanePreferences().visiblePanes().add(pane); - } - - private void onPaneRemoved(SidePaneType pane) { - stateManager.sidePaneComponentVisiblePropertyFor(pane).set(false); - preferencesService.getSidePanePreferences().visiblePanes().remove(pane); + protected SidePaneComponent getSidePaneComponent(SidePaneType pane) { + + SidePaneComponent sidePaneComponent = sidePaneComponentLookup.get(pane); + if (sidePaneComponent == null) { + sidePaneComponent = switch (pane) { + case GROUPS -> new GroupsSidePaneComponent( + new ClosePaneAction(pane), + new MoveUpAction(pane), + new MoveDownAction(pane), + sidePaneContentFactory, + preferencesService, + dialogService); + case WEB_SEARCH, OPEN_OFFICE -> new SidePaneComponent(pane, + new ClosePaneAction(pane), + new MoveUpAction(pane), + new MoveDownAction(pane), + sidePaneContentFactory); + }; + sidePaneComponentLookup.put(pane, sidePaneComponent); + } + return sidePaneComponent; } /** - * Stores the current configuration of visible panes in the preferences, - * so that we show panes at the preferred position next time. + * Stores the current configuration of visible panes in the preferences, so that we show panes at the preferred + * position next time. */ private void updatePreferredPositions() { Map preferredPositions = new HashMap<>(preferencesService.getSidePanePreferences().getPreferredPositions()); - IntStream.range(0, getVisiblePanes().size()).forEach(i -> preferredPositions.put(getVisiblePanes().get(i), i)); + IntStream.range(0, getPanes().size()).forEach(i -> preferredPositions.put(getPanes().get(i), i)); preferencesService.getSidePanePreferences().setPreferredPositions(preferredPositions); } - public ObservableList getVisiblePanes() { - return stateManager.getVisibleSidePaneComponents(); - } - - /** - * @return True if pane is visible, and it can still move up therefore we should update the view - */ - public boolean moveUp(SidePaneType pane) { - if (getVisiblePanes().contains(pane)) { - int currentPosition = getVisiblePanes().indexOf(pane); + public void moveUp(SidePaneType pane) { + if (getPanes().contains(pane)) { + int currentPosition = getPanes().indexOf(pane); if (currentPosition > 0) { int newPosition = currentPosition - 1; - swap(getVisiblePanes(), currentPosition, newPosition); + swap(getPanes(), currentPosition, newPosition); updatePreferredPositions(); - return true; + } else { + LOGGER.debug("SidePaneComponent is already at the bottom"); } + } else { + LOGGER.warn("SidePaneComponent {} not visible", pane.getTitle()); } - return false; } - /** - * @return True if pane is visible, and it can still move down therefore we should update the view - */ - public boolean moveDown(SidePaneType pane) { - if (getVisiblePanes().contains(pane)) { - int currentPosition = getVisiblePanes().indexOf(pane); - if (currentPosition < (getVisiblePanes().size() - 1)) { + public void moveDown(SidePaneType pane) { + if (getPanes().contains(pane)) { + int currentPosition = getPanes().indexOf(pane); + if (currentPosition < (getPanes().size() - 1)) { int newPosition = currentPosition + 1; - swap(getVisiblePanes(), currentPosition, newPosition); + swap(getPanes(), currentPosition, newPosition); updatePreferredPositions(); - return true; + } else { + LOGGER.debug("SidePaneComponent {} is already at the top", pane.getTitle()); } + } else { + LOGGER.warn("SidePaneComponent {} not visible", pane.getTitle()); } - return false; } - /** - * @return True if pane is not already shown which means the view needs to be updated - */ - public boolean show(SidePaneType pane) { - if (!getVisiblePanes().contains(pane)) { - getVisiblePanes().add(pane); - getVisiblePanes().sort(new PreferredIndexSort(preferencesService)); - return true; + public void show(SidePaneType pane) { + if (!getPanes().contains(pane)) { + getPanes().add(pane); + getPanes().sort(new PreferredIndexSort(preferencesService.getSidePanePreferences())); + } else { + LOGGER.warn("SidePaneComponent {} not visible", pane.getTitle()); + } + + if (pane == GROUPS + && stateManager.getVisibleSidePaneComponents().contains(GROUPS) + && getSidePaneComponent(pane) instanceof GroupsSidePaneComponent component) { + component.afterOpening(); } - return false; } - /** - * @return True if pane is visible which means the view needs to be updated - */ - public boolean hide(SidePaneType pane) { - if (getVisiblePanes().contains(pane)) { - getVisiblePanes().remove(pane); - return true; + public void hide(SidePaneType pane) { + if (getPanes().contains(pane)) { + getPanes().remove(pane); } else { - return false; + LOGGER.warn("SidePaneComponent {} not visible", pane.getTitle()); } } - public boolean isPaneVisible(SidePaneType pane) { - return getVisiblePanes().contains(pane); + private ObservableList getPanes() { + return stateManager.getVisibleSidePaneComponents(); } private void swap(ObservableList observableList, int i, int j) { @@ -127,12 +157,12 @@ private void swap(ObservableList observableList, int i, int j) { /** * Helper class for sorting visible side panes based on their preferred position. */ - private static class PreferredIndexSort implements Comparator { + protected static class PreferredIndexSort implements Comparator { private final Map preferredPositions; - public PreferredIndexSort(PreferencesService preferencesService) { - preferredPositions = preferencesService.getSidePanePreferences().getPreferredPositions(); + public PreferredIndexSort(SidePanePreferences sidePanePreferences) { + this.preferredPositions = sidePanePreferences.getPreferredPositions(); } @Override @@ -142,4 +172,43 @@ public int compare(SidePaneType type1, SidePaneType type2) { return Integer.compare(pos1, pos2); } } + + private class MoveUpAction extends SimpleCommand { + private final SidePaneType toMoveUpPane; + + public MoveUpAction(SidePaneType toMoveUpPane) { + this.toMoveUpPane = toMoveUpPane; + } + + @Override + public void execute() { + moveUp(toMoveUpPane); + } + } + + private class MoveDownAction extends SimpleCommand { + private final SidePaneType toMoveDownPane; + + public MoveDownAction(SidePaneType toMoveDownPane) { + this.toMoveDownPane = toMoveDownPane; + } + + @Override + public void execute() { + moveDown(toMoveDownPane); + } + } + + public class ClosePaneAction extends SimpleCommand { + private final SidePaneType toClosePane; + + public ClosePaneAction(SidePaneType toClosePane) { + this.toClosePane = toClosePane; + } + + @Override + public void execute() { + stateManager.getVisibleSidePaneComponents().remove(toClosePane); + } + } } diff --git a/src/main/java/org/jabref/gui/sidepane/TogglePaneAction.java b/src/main/java/org/jabref/gui/sidepane/TogglePaneAction.java index fccf961cab4..ab4ff37f2f2 100644 --- a/src/main/java/org/jabref/gui/sidepane/TogglePaneAction.java +++ b/src/main/java/org/jabref/gui/sidepane/TogglePaneAction.java @@ -2,19 +2,27 @@ import org.jabref.gui.StateManager; import org.jabref.gui.actions.SimpleCommand; +import org.jabref.preferences.SidePanePreferences; public class TogglePaneAction extends SimpleCommand { private final StateManager stateManager; private final SidePaneType pane; + private final SidePanePreferences sidePanePreferences; - public TogglePaneAction(StateManager stateManager, SidePaneType pane) { + public TogglePaneAction(StateManager stateManager, SidePaneType pane, SidePanePreferences sidePanePreferences) { this.stateManager = stateManager; this.pane = pane; + this.sidePanePreferences = sidePanePreferences; } @Override public void execute() { - boolean isVisible = stateManager.sidePaneComponentVisiblePropertyFor(pane).get(); - stateManager.sidePaneComponentVisiblePropertyFor(pane).set(!isVisible); + if (!stateManager.getVisibleSidePaneComponents().contains(pane)) { + stateManager.getVisibleSidePaneComponents().add(pane); + stateManager.getVisibleSidePaneComponents().sort(new SidePaneViewModel.PreferredIndexSort(sidePanePreferences)); + } else { + stateManager.getVisibleSidePaneComponents().remove(pane); + } + } } From 2149f6c4a9ed79fdf978bf1c3ea4912b185a03db Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage Date: Sun, 5 Dec 2021 23:17:59 +0100 Subject: [PATCH 30/36] Moved listener from sidepane visibility property to list of children in sidepane --- src/main/java/org/jabref/gui/JabRefFrame.java | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index 04c59948a21..5f694382285 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -12,6 +12,7 @@ import java.util.stream.Collectors; import javafx.application.Platform; +import javafx.beans.InvalidationListener; import javafx.beans.binding.Bindings; import javafx.beans.binding.StringBinding; import javafx.beans.value.ChangeListener; @@ -432,9 +433,12 @@ private void initLayout() { head.setSpacing(0d); setTop(head); - splitPane.getItems().addAll(sidePane, tabbedPane); + splitPane.getItems().addAll(tabbedPane); SplitPane.setResizableWithParent(sidePane, false); + sidePane.getChildren().addListener((InvalidationListener) c -> updateSidePane()); + updateSidePane(); + // We need to wait with setting the divider since it gets reset a few times during the initial set-up mainStage.showingProperty().addListener(new ChangeListener<>() { @@ -442,18 +446,6 @@ private void initLayout() { public void changed(ObservableValue observable, Boolean oldValue, Boolean showing) { if (showing) { setDividerPosition(); - EasyBind.subscribe(sidePane.visibleProperty(), visible -> { - if (visible) { - if (!splitPane.getItems().contains(sidePane)) { - splitPane.getItems().add(0, sidePane); - setDividerPosition(); - } - } else { - splitPane.getItems().remove(sidePane); - } - }); - - mainStage.showingProperty().removeListener(this); observable.removeListener(this); } } @@ -462,6 +454,17 @@ public void changed(ObservableValue observable, Boolean oldVa setCenter(splitPane); } + private void updateSidePane() { + if (sidePane.getChildren().isEmpty()) { + splitPane.getItems().remove(sidePane); + } else { + if (!splitPane.getItems().contains(sidePane)) { + splitPane.getItems().add(0, sidePane); + setDividerPosition(); + } + } + } + private void setDividerPosition() { splitPane.setDividerPositions(prefs.getGuiPreferences().getSidePaneWidth()); if (!splitPane.getDividers().isEmpty()) { From e74403c475fbd7c68d088e81f64a2572a0099db0 Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage Date: Mon, 6 Dec 2021 20:49:04 +0100 Subject: [PATCH 31/36] Removed unused method --- .../java/org/jabref/gui/sidepane/SidePaneViewModel.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java index 979289e149d..bfc4c310699 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java @@ -136,14 +136,6 @@ && getSidePaneComponent(pane) instanceof GroupsSidePaneComponent component) { } } - public void hide(SidePaneType pane) { - if (getPanes().contains(pane)) { - getPanes().remove(pane); - } else { - LOGGER.warn("SidePaneComponent {} not visible", pane.getTitle()); - } - } - private ObservableList getPanes() { return stateManager.getVisibleSidePaneComponents(); } From 531dafc543d0205992368a249efebf9335b42654 Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage Date: Sat, 11 Dec 2021 13:29:34 +0100 Subject: [PATCH 32/36] Fixed sidepane width issue on startup --- src/main/java/org/jabref/gui/JabRefFrame.java | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index 5f694382285..2a89967fbdd 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -13,10 +13,9 @@ import javafx.application.Platform; import javafx.beans.InvalidationListener; +import javafx.beans.Observable; import javafx.beans.binding.Bindings; import javafx.beans.binding.StringBinding; -import javafx.beans.value.ChangeListener; -import javafx.beans.value.ObservableValue; import javafx.collections.transformation.FilteredList; import javafx.concurrent.Task; import javafx.geometry.Orientation; @@ -143,6 +142,7 @@ import com.google.common.eventbus.Subscribe; import com.tobiasdiez.easybind.EasyBind; import com.tobiasdiez.easybind.EasyObservableList; +import com.tobiasdiez.easybind.Subscription; import org.controlsfx.control.PopOver; import org.controlsfx.control.TaskProgressView; import org.fxmisc.richtext.CodeArea; @@ -176,6 +176,8 @@ public class JabRefFrame extends BorderPane { private PopOver progressViewPopOver; private PopOver entryFromIdPopOver; + private Subscription dividerSubscription; + private final TaskExecutor taskExecutor; public JabRefFrame(Stage mainStage) { @@ -440,11 +442,10 @@ private void initLayout() { updateSidePane(); // We need to wait with setting the divider since it gets reset a few times during the initial set-up - mainStage.showingProperty().addListener(new ChangeListener<>() { - + mainStage.showingProperty().addListener(new InvalidationListener() { @Override - public void changed(ObservableValue observable, Boolean oldValue, Boolean showing) { - if (showing) { + public void invalidated(Observable observable) { + if (mainStage.isShowing()) { setDividerPosition(); observable.removeListener(this); } @@ -456,6 +457,9 @@ public void changed(ObservableValue observable, Boolean oldVa private void updateSidePane() { if (sidePane.getChildren().isEmpty()) { + if (dividerSubscription != null) { + dividerSubscription.unsubscribe(); + } splitPane.getItems().remove(sidePane); } else { if (!splitPane.getItems().contains(sidePane)) { @@ -467,8 +471,8 @@ private void updateSidePane() { private void setDividerPosition() { splitPane.setDividerPositions(prefs.getGuiPreferences().getSidePaneWidth()); - if (!splitPane.getDividers().isEmpty()) { - EasyBind.subscribe(splitPane.getDividers().get(0).positionProperty(), + if (mainStage.isShowing() && !sidePane.getChildren().isEmpty()) { + dividerSubscription = EasyBind.subscribe(splitPane.getDividers().get(0).positionProperty(), position -> prefs.getGuiPreferences().setSidePaneWidth(position.doubleValue())); } } From 1b56325ac84c827266caae777704638b4fcb080d Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage Date: Sat, 11 Dec 2021 17:55:29 +0100 Subject: [PATCH 33/36] Created tests --- .../gui/sidepane/SidePaneViewModelTest.java | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 src/test/java/org/jabref/gui/sidepane/SidePaneViewModelTest.java diff --git a/src/test/java/org/jabref/gui/sidepane/SidePaneViewModelTest.java b/src/test/java/org/jabref/gui/sidepane/SidePaneViewModelTest.java new file mode 100644 index 00000000000..03fb4dc6483 --- /dev/null +++ b/src/test/java/org/jabref/gui/sidepane/SidePaneViewModelTest.java @@ -0,0 +1,97 @@ +package org.jabref.gui.sidepane; + +import java.util.EnumSet; +import java.util.HashMap; +import java.util.HashSet; + +import javax.swing.undo.UndoManager; + +import javafx.collections.FXCollections; +import javafx.collections.ObservableList; + +import org.jabref.gui.DialogService; +import org.jabref.gui.StateManager; +import org.jabref.gui.util.CustomLocalDragboard; +import org.jabref.gui.util.OptionalObjectProperty; +import org.jabref.gui.util.TaskExecutor; +import org.jabref.preferences.PreferencesService; +import org.jabref.preferences.SidePanePreferences; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.testfx.framework.junit5.ApplicationExtension; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@ExtendWith(ApplicationExtension.class) +class SidePaneViewModelTest { + + PreferencesService preferencesService = mock(PreferencesService.class); + StateManager stateManager = mock(StateManager.class); + TaskExecutor taskExecutor = mock(TaskExecutor.class); + DialogService dialogService = mock(DialogService.class); + UndoManager undoManager = mock(UndoManager.class); + + SidePanePreferences sidePanePreferences = new SidePanePreferences(new HashSet<>(), new HashMap<>(), 0); + ObservableList sidePaneComponents = FXCollections.observableArrayList(); + SidePaneViewModel sidePaneViewModel; + + @BeforeEach + void setUp() { + when(stateManager.getVisibleSidePaneComponents()).thenReturn(sidePaneComponents); + when(stateManager.getLocalDragboard()).thenReturn(mock(CustomLocalDragboard.class)); + when(stateManager.activeDatabaseProperty()).thenReturn(OptionalObjectProperty.empty()); + when(preferencesService.getSidePanePreferences()).thenReturn(sidePanePreferences); + + sidePanePreferences.visiblePanes().addAll(EnumSet.allOf(SidePaneType.class)); + sidePanePreferences.getPreferredPositions().put(SidePaneType.GROUPS, 0); + sidePanePreferences.getPreferredPositions().put(SidePaneType.WEB_SEARCH, 1); + sidePanePreferences.getPreferredPositions().put(SidePaneType.OPEN_OFFICE, 2); + + sidePaneViewModel = new SidePaneViewModel(preferencesService, stateManager, taskExecutor, dialogService, undoManager); + } + + @Test + void moveUp() { + sidePaneViewModel.moveUp(SidePaneType.WEB_SEARCH); + + assertEquals(sidePaneComponents.get(0), SidePaneType.WEB_SEARCH); + assertEquals(sidePaneComponents.get(1), SidePaneType.GROUPS); + } + + @Test + void moveUpFromFirstPosition() { + sidePaneViewModel.moveUp(SidePaneType.GROUPS); + + assertEquals(sidePaneComponents.get(0), SidePaneType.GROUPS); + } + + @Test + void moveDown() { + sidePaneViewModel.moveDown(SidePaneType.WEB_SEARCH); + + assertEquals(sidePaneComponents.get(1), SidePaneType.OPEN_OFFICE); + assertEquals(sidePaneComponents.get(2), SidePaneType.WEB_SEARCH); + } + + @Test + void moveDownFromLastPosition() { + sidePaneViewModel.moveDown(SidePaneType.OPEN_OFFICE); + + assertEquals(sidePaneComponents.get(2), SidePaneType.OPEN_OFFICE); + } + + @Test + void sortByPreferredPositions() { + sidePanePreferences.getPreferredPositions().put(SidePaneType.GROUPS, 2); + sidePanePreferences.getPreferredPositions().put(SidePaneType.OPEN_OFFICE, 0); + + sidePaneComponents.sort(new SidePaneViewModel.PreferredIndexSort(sidePanePreferences)); + + assertTrue(sidePaneComponents.get(0) == SidePaneType.OPEN_OFFICE && sidePaneComponents.get(2) == SidePaneType.GROUPS); + } +} From 9af0d79ffc9c910dd6b437cd540367758e2b0118 Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage Date: Sat, 11 Dec 2021 18:04:23 +0100 Subject: [PATCH 34/36] Cleanups --- .../gui/sidepane/GroupsSidePaneComponent.java | 2 +- .../gui/sidepane/SidePaneComponent.java | 6 +++++- .../gui/sidepane/SidePaneContentFactory.java | 20 ++++++++++++++++--- .../gui/sidepane/SidePaneViewModel.java | 12 ++++++++--- .../jabref/gui/sidepane/TogglePaneAction.java | 1 - 5 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneComponent.java b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneComponent.java index bba07caf38a..b560c41524c 100644 --- a/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneComponent.java +++ b/src/main/java/org/jabref/gui/sidepane/GroupsSidePaneComponent.java @@ -28,7 +28,7 @@ public GroupsSidePaneComponent(SimpleCommand closeCommand, } private void setupIntersectionUnionToggle() { - addExtraButtonToHeader(intersectionUnionToggle, 2); + addExtraButtonToHeader(intersectionUnionToggle, 0); intersectionUnionToggle.setOnAction(event -> new ToggleUnionIntersectionAction().execute()); } diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java b/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java index 5580ecba8cb..5993f1a4c95 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneComponent.java @@ -22,7 +22,11 @@ public class SidePaneComponent extends BorderPane { private HBox buttonContainer; - public SidePaneComponent(SidePaneType sidePaneType, SimpleCommand closeCommand, SimpleCommand moveUpCommand, SimpleCommand moveDownCommand, SidePaneContentFactory contentFactory) { + public SidePaneComponent(SidePaneType sidePaneType, + SimpleCommand closeCommand, + SimpleCommand moveUpCommand, + SimpleCommand moveDownCommand, + SidePaneContentFactory contentFactory) { this.sidePaneType = sidePaneType; this.closeCommand = closeCommand; this.moveUpCommand = moveUpCommand; diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneContentFactory.java b/src/main/java/org/jabref/gui/sidepane/SidePaneContentFactory.java index bc32980765a..f3da681a510 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneContentFactory.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneContentFactory.java @@ -33,9 +33,23 @@ public SidePaneContentFactory(PreferencesService preferences, public Node create(SidePaneType sidePaneType) { return switch (sidePaneType) { - case GROUPS -> new GroupTreeView(taskExecutor, stateManager, preferences, dialogService); - case OPEN_OFFICE -> new OpenOfficePanel(preferences, preferences.getOpenOfficePreferences(), preferences.getKeyBindingRepository(), taskExecutor, dialogService, stateManager, undoManager).getContent(); - case WEB_SEARCH -> new WebSearchPaneView(preferences, dialogService, stateManager); + case GROUPS -> new GroupTreeView( + taskExecutor, + stateManager, + preferences, + dialogService); + case OPEN_OFFICE -> new OpenOfficePanel( + preferences, + preferences.getOpenOfficePreferences(), + preferences.getKeyBindingRepository(), + taskExecutor, + dialogService, + stateManager, + undoManager).getContent(); + case WEB_SEARCH -> new WebSearchPaneView( + preferences, + dialogService, + stateManager); }; } } diff --git a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java index bfc4c310699..8e661995dfa 100644 --- a/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java +++ b/src/main/java/org/jabref/gui/sidepane/SidePaneViewModel.java @@ -44,7 +44,12 @@ public SidePaneViewModel(PreferencesService preferencesService, this.preferencesService = preferencesService; this.stateManager = stateManager; this.dialogService = dialogService; - this.sidePaneContentFactory = new SidePaneContentFactory(preferencesService, taskExecutor, dialogService, stateManager, undoManager); + this.sidePaneContentFactory = new SidePaneContentFactory( + preferencesService, + taskExecutor, + dialogService, + stateManager, + undoManager); preferencesService.getSidePanePreferences().visiblePanes().forEach(this::show); getPanes().addListener((ListChangeListener) change -> { @@ -86,7 +91,8 @@ protected SidePaneComponent getSidePaneComponent(SidePaneType pane) { * position next time. */ private void updatePreferredPositions() { - Map preferredPositions = new HashMap<>(preferencesService.getSidePanePreferences().getPreferredPositions()); + Map preferredPositions = new HashMap<>(preferencesService.getSidePanePreferences() + .getPreferredPositions()); IntStream.range(0, getPanes().size()).forEach(i -> preferredPositions.put(getPanes().get(i), i)); preferencesService.getSidePanePreferences().setPreferredPositions(preferredPositions); } @@ -121,7 +127,7 @@ public void moveDown(SidePaneType pane) { } } - public void show(SidePaneType pane) { + private void show(SidePaneType pane) { if (!getPanes().contains(pane)) { getPanes().add(pane); getPanes().sort(new PreferredIndexSort(preferencesService.getSidePanePreferences())); diff --git a/src/main/java/org/jabref/gui/sidepane/TogglePaneAction.java b/src/main/java/org/jabref/gui/sidepane/TogglePaneAction.java index ab4ff37f2f2..087a53db78f 100644 --- a/src/main/java/org/jabref/gui/sidepane/TogglePaneAction.java +++ b/src/main/java/org/jabref/gui/sidepane/TogglePaneAction.java @@ -23,6 +23,5 @@ public void execute() { } else { stateManager.getVisibleSidePaneComponents().remove(pane); } - } } From ecfb5290ed0cb6d3dd1f58ae662fdec419b0b6a1 Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage Date: Sat, 11 Dec 2021 18:10:52 +0100 Subject: [PATCH 35/36] CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8d88cebc9e..a389c14334e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - We reworked the export order in the preferences and the save order in the library preferences. You can now set more than three sort criteria in your library preferences. [#7935](https://github.com/JabRef/jabref/pull/7935) - The metadata-to-pdf actions now also embeds the bibfile to the PDF. [#8037](https://github.com/JabRef/jabref/pull/8037) - The snap was updated to use the core20 base and to use lzo compression for better startup performance [#8109](https://github.com/JabRef/jabref/pull/8109) +- We moved the union/intersection view button in the group sidepane to the left of the other controls. [#8202](https://github.com/JabRef/jabref/pull/8202) - We improved the Drag and Drop behavior in the "Customize Entry Types" Dialog [#6338](https://github.com/JabRef/jabref/issues/6338) - When determining the URL of an ArXiV eprint, the URL now points to the version [#8149](https://github.com/JabRef/jabref/pull/8149) - We Included all standard fields with citation key when exporting to Old OpenOffice/LibreOffice Calc Format [#8176](https://github.com/JabRef/jabref/pull/8176) @@ -61,6 +62,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - We fixed some icons that were drawn in the wrong color when JabRef used a custom theme. [#7853](https://github.com/JabRef/jabref/issues/7853) - We fixed an issue where the `Aux file` on `Edit group` doesn't support relative sub-directories path to import. [#7719](https://github.com/JabRef/jabref/issues/7719). - We fixed an issue where it was impossible to add or modify groups. [#7912](https://github.com/JabRef/jabref/pull/793://github.com/JabRef/jabref/pull/7921) +- We fixed an issue about the visible side pane components being out of sync with the view menu. [#8115](https://github.com/JabRef/jabref/issues/8115) - We fixed an issue where exported entries from a Citavi bib containing URLs could not be imported [#7892](https://github.com/JabRef/jabref/issues/7882) - We fixed an issue where the icons in the search bar had the same color, toggled as well as untoggled. [#8014](https://github.com/JabRef/jabref/pull/8014) - We fixed an issue where typing an invalid UNC path into the "Main file directory" text field caused an error. [#8107](https://github.com/JabRef/jabref/issues/8107) @@ -73,6 +75,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - We fixed an issue where selecting a citation style in the preferences would sometimes produce an exception [#7860](https://github.com/JabRef/jabref/issues/7860) - We fixed an issue where an exception would occur when clicking on a DOI link in the preview pane [#7706](https://github.com/JabRef/jabref/issues/7706) + ### Removed - We removed two orphaned preferences options [#8164](https://github.com/JabRef/jabref/pull/8164) From 2ff238b5f9851f76156df3656a8b167deb79ec6c Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage Date: Sat, 11 Dec 2021 18:13:51 +0100 Subject: [PATCH 36/36] more CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a389c14334e..300f712383b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - We fixed an issue where the `Aux file` on `Edit group` doesn't support relative sub-directories path to import. [#7719](https://github.com/JabRef/jabref/issues/7719). - We fixed an issue where it was impossible to add or modify groups. [#7912](https://github.com/JabRef/jabref/pull/793://github.com/JabRef/jabref/pull/7921) - We fixed an issue about the visible side pane components being out of sync with the view menu. [#8115](https://github.com/JabRef/jabref/issues/8115) +- We fixed an issue where the side pane would not close when all its components were closed. [#8082]((https://github.com/JabRef/jabref/issues/8082)) - We fixed an issue where exported entries from a Citavi bib containing URLs could not be imported [#7892](https://github.com/JabRef/jabref/issues/7882) - We fixed an issue where the icons in the search bar had the same color, toggled as well as untoggled. [#8014](https://github.com/JabRef/jabref/pull/8014) - We fixed an issue where typing an invalid UNC path into the "Main file directory" text field caused an error. [#8107](https://github.com/JabRef/jabref/issues/8107)