Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Sidepane logic #8202

Merged
merged 37 commits into from
Dec 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
13fc122
Add visibleProperty to SidePaneComponent
HoussemNasri Oct 30, 2021
9688ec6
Making progress into refactoring Sidepane logic
HoussemNasri Oct 31, 2021
e7ddff6
Populate SidePaneType with information
HoussemNasri Oct 31, 2021
eee68e7
Wrap the content of web search side pane into WebSearchPaneView
HoussemNasri Oct 31, 2021
9c3a314
Revert changes to SidePane
HoussemNasri Oct 31, 2021
f59bb3f
Refactor SidePaneContainer
HoussemNasri Oct 31, 2021
41dba61
Bind side pane check menu selection state to side pane visibility
HoussemNasri Oct 31, 2021
c839950
Fix checkstyle
HoussemNasri Oct 31, 2021
69c90df
Remove SidePaneViewModel.java
HoussemNasri Oct 31, 2021
a9e0419
Fix view menu for sidebar option out of sync with sidebar state
HoussemNasri Oct 31, 2021
289165b
Remove unused constructor parameters
HoussemNasri Oct 31, 2021
d807d9b
Remove deprecated Sidepane component
HoussemNasri Oct 31, 2021
971203a
Move Vbox.setVgrow() to SidePaneView
HoussemNasri Nov 1, 2021
2266a67
Merge SidePaneHeaderView into SidePaneView
HoussemNasri Nov 1, 2021
8224cc5
Avoid firing removed and added events on ObservableList
HoussemNasri Nov 2, 2021
5a877ab
Use shorter more declarative code for binding
HoussemNasri Nov 2, 2021
1d35778
Rename SidePaneView to SidePaneComponent and SidePaneContainerView to…
HoussemNasri Nov 6, 2021
6d9d52d
Rename initView() to initialize()
HoussemNasri Nov 6, 2021
2f0590d
Add an overloading of createCheckMenuItem() with a selected property
HoussemNasri Nov 6, 2021
58edb1c
Rename sidePaneVisibleProperty() to paneVisibleProperty()
HoussemNasri Nov 6, 2021
7e70c25
Use the name pane rather than sidePane to call components in the Side…
HoussemNasri Nov 6, 2021
9cd8894
Remove unused method createSidePaneCheckMenuItem()
HoussemNasri Nov 6, 2021
7176862
Share the visibility property of side pane components by moving it to…
HoussemNasri Nov 6, 2021
b0cae97
Remove comment
HoussemNasri Nov 6, 2021
85308f7
Use better names
HoussemNasri Nov 6, 2021
e44ae48
Fix checkstyle
HoussemNasri Nov 6, 2021
2616335
Move close/toggle pane actions to separate files
HoussemNasri Nov 8, 2021
ab49960
Move sidepane binding logic to SidePaneViewModel
HoussemNasri Nov 14, 2021
e2141bc
Merge remote-tracking branch 'upstream/main' into refactor-sidepane
calixtus Dec 1, 2021
e573811
Removed unnecessary properties in stateManager, refactored to mvvm pa…
calixtus Dec 5, 2021
2149f6c
Moved listener from sidepane visibility property to list of children …
calixtus Dec 5, 2021
e74403c
Removed unused method
calixtus Dec 6, 2021
531dafc
Fixed sidepane width issue on startup
calixtus Dec 11, 2021
1b56325
Created tests
calixtus Dec 11, 2021
9af0d79
Cleanups
calixtus Dec 11, 2021
ecfb529
CHANGELOG.md
calixtus Dec 11, 2021
2ff238b
more CHANGELOG.md
calixtus Dec 11, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -61,6 +62,8 @@ 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 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)
Expand All @@ -73,6 +76,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)
Expand Down
72 changes: 37 additions & 35 deletions src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
import java.util.stream.Collectors;

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;
Expand Down Expand Up @@ -109,7 +109,6 @@
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.SidePaneType;
import org.jabref.gui.slr.ExistingStudySearchAction;
import org.jabref.gui.slr.StartNewStudyAction;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -341,7 +343,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<String> filenames) {
if (prefs.getGuiPreferences().shouldOpenLastEdited()) {
Expand Down Expand Up @@ -433,29 +435,18 @@ private void initLayout() {
head.setSpacing(0d);
setTop(head);

splitPane.getItems().addAll(sidePane, tabbedPane);
splitPane.getItems().addAll(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<>() {
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 InvalidationListener() {
@Override
public void changed(ObservableValue<? extends Boolean> observable, Boolean oldValue, Boolean showing) {
if (showing) {
public void invalidated(Observable observable) {
if (mainStage.isShowing()) {
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);
}
}
Expand All @@ -464,10 +455,24 @@ public void changed(ObservableValue<? extends Boolean> observable, Boolean oldVa
setCenter(splitPane);
}

private void updateSidePane() {
if (sidePane.getChildren().isEmpty()) {
if (dividerSubscription != null) {
dividerSubscription.unsubscribe();
}
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()) {
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()));
}
}
Expand Down Expand Up @@ -577,7 +582,6 @@ public void showLibraryTab(LibraryTab libraryTab) {

public void init() {
sidePane = new SidePane(prefs, taskExecutor, dialogService, stateManager, undoManager);

tabbedPane = new TabPane();
tabbedPane.setTabDragPolicy(TabPane.TabDragPolicy.REORDER);

Expand Down Expand Up @@ -838,15 +842,13 @@ 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);

SidePaneType webSearchPane = SidePaneType.WEB_SEARCH;
SidePaneType groupsPane = SidePaneType.GROUPS;
SidePaneType openOfficePane = 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)),
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(),

Expand Down Expand Up @@ -1111,7 +1113,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) {
Expand Down Expand Up @@ -1291,7 +1293,7 @@ public CloseDatabaseAction(LibraryTab libraryTab) {

/**
* Using this constructor will result in executing the command on the currently open library tab
* */
*/
public CloseDatabaseAction() {
this(null);
}
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/org/jabref/gui/StateManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
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;
Expand Down Expand Up @@ -56,11 +57,16 @@ public class StateManager {
private final EasyBinding<Boolean> anyTaskRunning = EasyBind.reduce(backgroundTasks, tasks -> tasks.anyMatch(Task::isRunning));
private final EasyBinding<Double> tasksProgress = EasyBind.reduce(backgroundTasks, tasks -> tasks.filter(Task::isRunning).mapToDouble(Task::getProgress).average().orElse(1));
private final ObservableMap<String, DialogWindowState> dialogWindowStates = FXCollections.observableHashMap();
private final ObservableList<SidePaneType> visibleSidePanes = FXCollections.observableArrayList();

public StateManager() {
activeGroups.bind(Bindings.valueAt(selectedGroups, activeDatabase.orElse(null)));
}

public ObservableList<SidePaneType> getVisibleSidePaneComponents() {
return visibleSidePanes;
}

public CustomLocalDragboard getLocalDragboard() {
return localDragboard;
}
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/org/jabref/gui/actions/ActionFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.lang.reflect.Method;
import java.util.Objects;

import javafx.beans.binding.BooleanExpression;
import javafx.scene.control.Button;
import javafx.scene.control.ButtonBase;
import javafx.scene.control.CheckMenuItem;
Expand Down Expand Up @@ -113,6 +114,14 @@ public CheckMenuItem createCheckMenuItem(Action action, Command command, boolean
return checkMenuItem;
}

public CheckMenuItem createCheckMenuItem(Action action, Command command, BooleanExpression selectedBinding) {
CheckMenuItem checkMenuItem = ActionUtils.createCheckMenuItem(new JabRefAction(action, command, keyBindingRepository, Sources.FromMenu));
EasyBind.subscribe(selectedBinding, checkMenuItem::setSelected);
setGraphic(checkMenuItem, action);

return checkMenuItem;
}

public Menu createMenu(Action action) {
Menu menu = ActionUtils.createMenu(new JabRefAction(action, keyBindingRepository));

Expand Down
97 changes: 0 additions & 97 deletions src/main/java/org/jabref/gui/groups/GroupSidePane.java

This file was deleted.

Loading