From 910c857b0211379da7db78995d61064bd839ba42 Mon Sep 17 00:00:00 2001 From: m-peeler Date: Fri, 5 Jul 2024 02:58:07 -0400 Subject: [PATCH 01/24] Auto-Select Newly Created Group When a new group is added, that group will automatically be selected and focused. --- src/main/java/org/jabref/gui/StateManager.java | 1 + src/main/java/org/jabref/gui/groups/GroupTreeView.java | 5 ++++- src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java | 4 ++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jabref/gui/StateManager.java b/src/main/java/org/jabref/gui/StateManager.java index f2d2b955f80..283702eec08 100644 --- a/src/main/java/org/jabref/gui/StateManager.java +++ b/src/main/java/org/jabref/gui/StateManager.java @@ -140,6 +140,7 @@ public void setSelectedEntries(List newSelectedEntries) { public void setSelectedGroups(BibDatabaseContext database, List newSelectedGroups) { Objects.requireNonNull(newSelectedGroups); + selectedGroups.clear(); selectedGroups.put(database.getUid(), FXCollections.observableArrayList(newSelectedGroups)); } diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeView.java b/src/main/java/org/jabref/gui/groups/GroupTreeView.java index a200d03c045..8bae9b4353c 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeView.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeView.java @@ -175,7 +175,10 @@ private void initialize() { BindingsHelper.bindContentBidirectional( groupTree.getSelectionModel().getSelectedItems(), viewModel.selectedGroupsProperty(), - newSelectedGroups -> newSelectedGroups.forEach(this::selectNode), + newSelectedGroups -> { + groupTree.getSelectionModel().clearSelection(); + newSelectedGroups.forEach(this::selectNode); + }, this::updateSelection )); diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java index de0c579699b..ae7dcc241fd 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java @@ -178,7 +178,8 @@ public void addNewSubgroup(GroupNodeViewModel parent, GroupDialogHeader groupDia groupDialogHeader)); newGroup.ifPresent(group -> { - parent.addSubgroup(group); + GroupTreeNode newSubgroup = parent.addSubgroup(group); + selectedGroups.setAll(new GroupNodeViewModel(database, stateManager, taskExecutor, newSubgroup, localDragboard, preferences)); // TODO: Add undo // UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(parent, new GroupTreeNodeViewModel(newGroupNode), UndoableAddOrRemoveGroup.ADD_NODE); @@ -186,7 +187,6 @@ public void addNewSubgroup(GroupNodeViewModel parent, GroupDialogHeader groupDia // TODO: Expand parent to make new group visible // parent.expand(); - dialogService.notify(Localization.lang("Added group \"%0\".", group.getName())); writeGroupChangesToMetaData(); }); From cbc0d7d977472325685d1139307552c37c9f7a06 Mon Sep 17 00:00:00 2001 From: m-peeler Date: Fri, 5 Jul 2024 03:05:47 -0400 Subject: [PATCH 02/24] Update CHANGELOG.md Updated changelog to reflect updates. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b6c5291bcb..24568329db6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - JabRef no longer downloads HTML files of websites when a PDF was not found. [#10149](https://github.com/JabRef/jabref/issues/10149) - We added the HTTP message (in addition to the response code) if an error is encountered. [#11341](https://github.com/JabRef/jabref/pull/11341) - We made label wrap text to fit view size when reviewing external group changes. [#11220](https://github.com/JabRef/jabref/issues/11220) +- We made new Groups automatically focus upon creation. [#11449](https://github.com/JabRef/jabref/issues/11449) ### Fixed From f126974d1d1bc5db087bf982ba3e91603c838a38 Mon Sep 17 00:00:00 2001 From: m-peeler Date: Fri, 5 Jul 2024 11:37:22 -0400 Subject: [PATCH 03/24] Implemented "Current selection" group option Added new option to the "Group Options" in the GroupDialog to allow the creation of a group that contains the currently selected entries. If there are no entries selected, this option is disabled. If there is more than one entry selected, this option is selected by default. --- CHANGELOG.md | 1 + .../org/jabref/gui/groups/GroupDialog.fxml | 6 +++ .../jabref/gui/groups/GroupDialogView.java | 11 ++++- .../gui/groups/GroupDialogViewModel.java | 46 ++++++++++++++++++- .../org/jabref/gui/groups/GroupTreeView.java | 2 +- .../jabref/gui/groups/GroupTreeViewModel.java | 8 +++- src/main/resources/l10n/JabRef_en.properties | 2 + 7 files changed, 69 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b6c5291bcb..f2e9c226503 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - We added an exporter and improved the importer for Endnote XML format. [#11137](https://github.com/JabRef/jabref/issues/11137) - We added support for using BibTeX Style files (BST) in the Preview. [#11102](https://github.com/JabRef/jabref/issues/11102) - We added support for automatically update LaTeX citations when a LaTeX file is created, removed, or modified. [#10585](https://github.com/JabRef/jabref/issues/10585) +- We added the ability to collect a group by your current selection. [#11449](https://github.com/JabRef/jabref/issues/11449) ### Changed diff --git a/src/main/java/org/jabref/gui/groups/GroupDialog.fxml b/src/main/java/org/jabref/gui/groups/GroupDialog.fxml index 02b19876250..cb26bcf91e4 100644 --- a/src/main/java/org/jabref/gui/groups/GroupDialog.fxml +++ b/src/main/java/org/jabref/gui/groups/GroupDialog.fxml @@ -99,6 +99,12 @@ + + + + + diff --git a/src/main/java/org/jabref/gui/groups/GroupDialogView.java b/src/main/java/org/jabref/gui/groups/GroupDialogView.java index b06c26503b1..4a0d72e0cd3 100644 --- a/src/main/java/org/jabref/gui/groups/GroupDialogView.java +++ b/src/main/java/org/jabref/gui/groups/GroupDialogView.java @@ -41,6 +41,7 @@ import org.jabref.logic.help.HelpFile; import org.jabref.logic.l10n.Localization; import org.jabref.model.database.BibDatabaseContext; +import org.jabref.model.entry.BibEntry; import org.jabref.model.groups.AbstractGroup; import org.jabref.model.groups.GroupHierarchyType; import org.jabref.model.groups.GroupTreeNode; @@ -81,6 +82,7 @@ public class GroupDialogView extends BaseDialog { @FXML private RadioButton searchRadioButton; @FXML private RadioButton autoRadioButton; @FXML private RadioButton texRadioButton; + @FXML private RadioButton selectionRadioButton; // Option Groups @FXML private TextField keywordGroupSearchTerm; @@ -109,6 +111,7 @@ public class GroupDialogView extends BaseDialog { private final BibDatabaseContext currentDatabase; private final @Nullable GroupTreeNode parentNode; private final @Nullable AbstractGroup editedGroup; + private final List selectedEntries; private GroupDialogViewModel viewModel; @@ -119,10 +122,12 @@ public class GroupDialogView extends BaseDialog { public GroupDialogView(BibDatabaseContext currentDatabase, @Nullable GroupTreeNode parentNode, @Nullable AbstractGroup editedGroup, - GroupDialogHeader groupDialogHeader) { + GroupDialogHeader groupDialogHeader, + List selectedEntries) { this.currentDatabase = currentDatabase; this.parentNode = parentNode; this.editedGroup = editedGroup; + this.selectedEntries = selectedEntries; ViewLoader.view(this) .load() @@ -172,7 +177,7 @@ public GroupDialogView(BibDatabaseContext currentDatabase, @FXML public void initialize() { - viewModel = new GroupDialogViewModel(dialogService, currentDatabase, preferencesService, editedGroup, parentNode, fileUpdateMonitor); + viewModel = new GroupDialogViewModel(dialogService, currentDatabase, preferencesService, editedGroup, parentNode, fileUpdateMonitor, selectedEntries); setResultConverter(viewModel::resultConverter); @@ -200,6 +205,8 @@ public void initialize() { searchRadioButton.selectedProperty().bindBidirectional(viewModel.typeSearchProperty()); autoRadioButton.selectedProperty().bindBidirectional(viewModel.typeAutoProperty()); texRadioButton.selectedProperty().bindBidirectional(viewModel.typeTexProperty()); + selectionRadioButton.selectedProperty().bindBidirectional(viewModel.typeSelectionProperty()); + selectionRadioButton.disableProperty().bind(viewModel.entriesAreSelectedProperty().not()); keywordGroupSearchTerm.textProperty().bindBidirectional(viewModel.keywordGroupSearchTermProperty()); keywordGroupSearchField.textProperty().bindBidirectional(viewModel.keywordGroupSearchFieldProperty()); diff --git a/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java b/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java index 73f671e65e2..9064b880f2f 100644 --- a/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java @@ -33,6 +33,7 @@ import org.jabref.logic.util.io.FileUtil; import org.jabref.model.database.BibDatabase; import org.jabref.model.database.BibDatabaseContext; +import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.Keyword; import org.jabref.model.entry.field.FieldFactory; import org.jabref.model.groups.AbstractGroup; @@ -76,6 +77,7 @@ public class GroupDialogViewModel { private final BooleanProperty typeSearchProperty = new SimpleBooleanProperty(); private final BooleanProperty typeAutoProperty = new SimpleBooleanProperty(); private final BooleanProperty typeTexProperty = new SimpleBooleanProperty(); + private final BooleanProperty typeSelectionProperty = new SimpleBooleanProperty(); // Option Groups private final StringProperty keywordGroupSearchTermProperty = new SimpleStringProperty(""); @@ -94,6 +96,7 @@ public class GroupDialogViewModel { private final StringProperty autoGroupPersonsFieldProperty = new SimpleStringProperty(""); private final StringProperty texGroupFilePathProperty = new SimpleStringProperty(""); + private final BooleanProperty entriesAreSelected = new SimpleBooleanProperty(false); private Validator nameValidator; private Validator nameContainsDelimiterValidator; @@ -112,24 +115,38 @@ public class GroupDialogViewModel { private final AbstractGroup editedGroup; private final GroupTreeNode parentNode; private final FileUpdateMonitor fileUpdateMonitor; + private final List selectedEntries; public GroupDialogViewModel(DialogService dialogService, BibDatabaseContext currentDatabase, PreferencesService preferencesService, @Nullable AbstractGroup editedGroup, @Nullable GroupTreeNode parentNode, - FileUpdateMonitor fileUpdateMonitor) { + FileUpdateMonitor fileUpdateMonitor, + List selectedEntries) { this.dialogService = dialogService; this.preferencesService = preferencesService; this.currentDatabase = currentDatabase; this.editedGroup = editedGroup; this.parentNode = parentNode; this.fileUpdateMonitor = fileUpdateMonitor; + this.selectedEntries = selectedEntries; setupValidation(); setValues(); } + public GroupDialogViewModel ( + DialogService dialogService, + BibDatabaseContext currentDatabase, + PreferencesService preferencesService, + @Nullable AbstractGroup editedGroup, + @Nullable GroupTreeNode parentNode, + FileUpdateMonitor fileUpdateMonitor + ) { + this(dialogService, currentDatabase, preferencesService, editedGroup, parentNode, fileUpdateMonitor, new ArrayList<>()); + } + private void setupValidation() { validator = new CompositeValidator(); @@ -371,6 +388,14 @@ public AbstractGroup resultConverter(ButtonType button) { new DefaultAuxParser(new BibDatabase()), fileUpdateMonitor, currentDatabase.getMetaData()); + } else if (typeSelectionProperty.getValue()) { + ExplicitGroup tempResultingGroup = new ExplicitGroup( + groupName, + groupHierarchySelectedProperty.getValue(), + preferencesService.getBibEntryPreferences().getKeywordSeparator() + ); + tempResultingGroup.add(selectedEntries); + resultingGroup = tempResultingGroup; } if (resultingGroup != null) { @@ -405,7 +430,16 @@ public void setValues() { .ifPresent(iconProperty::setValue); parentNode.getGroup().getColor().ifPresent(color -> colorUseProperty.setValue(true)); } - typeExplicitProperty.setValue(true); + if (!selectedEntries.isEmpty()) { + entriesAreSelected.setValue(true); + if (selectedEntries.size() > 1) { + typeSelectionProperty.setValue(true); + } else { + typeExplicitProperty.setValue(true); + } + } else { + typeExplicitProperty.setValue(true); + } groupHierarchySelectedProperty.setValue(preferencesService.getGroupsPreferences().getDefaultHierarchicalContext()); autoGroupKeywordsOptionProperty.setValue(Boolean.TRUE); } else { @@ -587,6 +621,10 @@ public BooleanProperty typeTexProperty() { return typeTexProperty; } + public BooleanProperty typeSelectionProperty() { + return typeSelectionProperty; + } + public StringProperty keywordGroupSearchTermProperty() { return keywordGroupSearchTermProperty; } @@ -638,4 +676,8 @@ public StringProperty autoGroupPersonsFieldProperty() { public StringProperty texGroupFilePathProperty() { return texGroupFilePathProperty; } + + public BooleanProperty entriesAreSelectedProperty() { + return entriesAreSelected; + } } diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeView.java b/src/main/java/org/jabref/gui/groups/GroupTreeView.java index a200d03c045..814ae2ed4e8 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeView.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeView.java @@ -399,7 +399,7 @@ private void updateSelection(List> newSelectedGroup } private void selectNode(GroupNodeViewModel value) { - selectNode(value, false); + selectNode(value, true); } private void selectNode(GroupNodeViewModel value, boolean expandParents) { diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java index de0c579699b..4dbbc4d68f4 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java @@ -175,7 +175,9 @@ public void addNewSubgroup(GroupNodeViewModel parent, GroupDialogHeader groupDia database, parent.getGroupNode(), null, - groupDialogHeader)); + groupDialogHeader, + stateManager.getSelectedEntries() + )); newGroup.ifPresent(group -> { parent.addSubgroup(group); @@ -260,7 +262,9 @@ public void editGroup(GroupNodeViewModel oldGroup) { database, oldGroup.getGroupNode().getParent().orElse(null), oldGroup.getGroupNode().getGroup(), - GroupDialogHeader.SUBGROUP)); + GroupDialogHeader.SUBGROUP, + stateManager.getSelectedEntries() + )); newGroup.ifPresent(group -> { AbstractGroup oldGroupDef = oldGroup.getGroupNode().getGroup(); diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index 70307993aca..b6b7fcf35b0 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -2649,3 +2649,5 @@ Note\:\ The\ study\ directory\ should\ be\ empty.=Note: The study directory shou Warning\:\ The\ selected\ directory\ is\ not\ empty.=Warning: The selected directory is not empty. Warning\:\ Failed\ to\ check\ if\ the\ directory\ is\ empty.=Warning: Failed to check if the directory is empty. Warning\:\ The\ selected\ directory\ is\ not\ a\ valid\ directory.=Warning: The selected directory is not a valid directory. +Current\ selection=Current selection +Group\ all\ entries\ currently\ selected=Group containing all entries currently selected From e4c2281dfa066c60409c89ccb0195c0c030a65f2 Mon Sep 17 00:00:00 2001 From: m-peeler Date: Fri, 5 Jul 2024 12:01:20 -0400 Subject: [PATCH 04/24] Addressed Style and Property Name Errors Fixes failed unit test (keyValueShouldBeEqualForEnglishPropertiesMessages) and style test ( '(' is preceded with whitespace.) --- src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java | 2 +- src/main/resources/l10n/JabRef_en.properties | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java b/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java index 9064b880f2f..20cee1d176a 100644 --- a/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java @@ -136,7 +136,7 @@ public GroupDialogViewModel(DialogService dialogService, setValues(); } - public GroupDialogViewModel ( + public GroupDialogViewModel( DialogService dialogService, BibDatabaseContext currentDatabase, PreferencesService preferencesService, diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index b6b7fcf35b0..e4f5facb83f 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -2650,4 +2650,4 @@ Warning\:\ The\ selected\ directory\ is\ not\ empty.=Warning: The selected direc Warning\:\ Failed\ to\ check\ if\ the\ directory\ is\ empty.=Warning: Failed to check if the directory is empty. Warning\:\ The\ selected\ directory\ is\ not\ a\ valid\ directory.=Warning: The selected directory is not a valid directory. Current\ selection=Current selection -Group\ all\ entries\ currently\ selected=Group containing all entries currently selected +Group\ all\ entries\ currently\ selected=Group all entries currently selected From 9b4d2db99ba18c47e721ccce3c73ef754fa543d4 Mon Sep 17 00:00:00 2001 From: m-peeler Date: Fri, 5 Jul 2024 14:18:46 -0400 Subject: [PATCH 05/24] Created 'Create Group From Selection' action Created the 'Create group from Seletion' item for the right-click menu and a new item on the menu. Began refactoring GroupDialogView, GroupDialogViewModel and GroupTreeViewModel to allow for creating a DialogView that automatically prioritizes 'Current selection'. --- .../jabref/gui/actions/StandardActions.java | 1 + .../jabref/gui/groups/GroupDialogView.java | 18 ++++++++- .../gui/groups/GroupDialogViewModel.java | 30 ++++++++++++++- .../jabref/gui/groups/GroupTreeViewModel.java | 28 +++++++++++--- .../CreateGroupFromSelectionAction.java | 37 +++++++++++++++++++ .../jabref/gui/maintable/RightClickMenu.java | 2 + 6 files changed, 106 insertions(+), 10 deletions(-) create mode 100644 src/main/java/org/jabref/gui/maintable/CreateGroupFromSelectionAction.java diff --git a/src/main/java/org/jabref/gui/actions/StandardActions.java b/src/main/java/org/jabref/gui/actions/StandardActions.java index 931e049d163..cd9a3a0c3f7 100644 --- a/src/main/java/org/jabref/gui/actions/StandardActions.java +++ b/src/main/java/org/jabref/gui/actions/StandardActions.java @@ -36,6 +36,7 @@ public enum StandardActions implements Action { EXTRACT_FILE_REFERENCES_OFFLINE(Localization.lang("Extract references from file (offline)"), IconTheme.JabRefIcons.FILE_STAR), OPEN_URL(Localization.lang("Open URL or DOI"), IconTheme.JabRefIcons.WWW, KeyBinding.OPEN_URL_OR_DOI), SEARCH_SHORTSCIENCE(Localization.lang("Search ShortScience")), + CREATE_GROUP_FROM_SELECTION(Localization.lang("Create group from selection")), MERGE_WITH_FETCHED_ENTRY(Localization.lang("Get bibliographic data from %0", "DOI/ISBN/...")), ATTACH_FILE(Localization.lang("Attach file"), IconTheme.JabRefIcons.ATTACH_FILE), ATTACH_FILE_FROM_URL(Localization.lang("Attach file from URL"), IconTheme.JabRefIcons.DOWNLOAD_FILE), diff --git a/src/main/java/org/jabref/gui/groups/GroupDialogView.java b/src/main/java/org/jabref/gui/groups/GroupDialogView.java index b06c26503b1..e3c6f4294c9 100644 --- a/src/main/java/org/jabref/gui/groups/GroupDialogView.java +++ b/src/main/java/org/jabref/gui/groups/GroupDialogView.java @@ -109,6 +109,8 @@ public class GroupDialogView extends BaseDialog { private final BibDatabaseContext currentDatabase; private final @Nullable GroupTreeNode parentNode; private final @Nullable AbstractGroup editedGroup; + private final List selectedEntries; + private final boolean useSelectedEntries; private GroupDialogViewModel viewModel; @@ -119,10 +121,22 @@ public class GroupDialogView extends BaseDialog { public GroupDialogView(BibDatabaseContext currentDatabase, @Nullable GroupTreeNode parentNode, @Nullable AbstractGroup editedGroup, - GroupDialogHeader groupDialogHeader) { + GroupDialogHeader groupDialogHeader, + List selectedEntries) { + this(currentDatabase, parentNode, editedGroup, groupDialogHeader, selectedEntries, false); + } + + public GroupDialogView(BibDatabaseContext currentDatabase, + @Nullable GroupTreeNode parentNode, + @Nullable AbstractGroup editedGroup, + GroupDialogHeader groupDialogHeader, + List selectedEntries, + boolean useSelectedEntries) { this.currentDatabase = currentDatabase; this.parentNode = parentNode; this.editedGroup = editedGroup; + this.selectedEntries = selectedEntries; + this.useSelectedEntries = useSelectedEntries; ViewLoader.view(this) .load() @@ -172,7 +186,7 @@ public GroupDialogView(BibDatabaseContext currentDatabase, @FXML public void initialize() { - viewModel = new GroupDialogViewModel(dialogService, currentDatabase, preferencesService, editedGroup, parentNode, fileUpdateMonitor); + viewModel = new GroupDialogViewModel(dialogService, currentDatabase, preferencesService, editedGroup, parentNode, fileUpdateMonitor, selectedEntries, useSelectedEntries); setResultConverter(viewModel::resultConverter); diff --git a/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java b/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java index 73f671e65e2..b29f0ae05ab 100644 --- a/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java @@ -112,24 +112,41 @@ public class GroupDialogViewModel { private final AbstractGroup editedGroup; private final GroupTreeNode parentNode; private final FileUpdateMonitor fileUpdateMonitor; + private final List selectedEntries; + private final boolean useSelectedEntries; public GroupDialogViewModel(DialogService dialogService, BibDatabaseContext currentDatabase, PreferencesService preferencesService, @Nullable AbstractGroup editedGroup, @Nullable GroupTreeNode parentNode, - FileUpdateMonitor fileUpdateMonitor) { + FileUpdateMonitor fileUpdateMonitor, + List selectedEntries, + boolean useSelectedEntries) { this.dialogService = dialogService; this.preferencesService = preferencesService; this.currentDatabase = currentDatabase; this.editedGroup = editedGroup; this.parentNode = parentNode; this.fileUpdateMonitor = fileUpdateMonitor; + this.selectedEntries = selectedEntries; + this.useSelectedEntries = useSelectedEntries; setupValidation(); setValues(); } + public GroupDialogViewModel( + DialogService dialogService, + BibDatabaseContext currentDatabase, + PreferencesService preferencesService, + @Nullable AbstractGroup editedGroup, + @Nullable GroupTreeNode parentNode, + FileUpdateMonitor fileUpdateMonitor + ) { + this(dialogService, currentDatabase, preferencesService, editedGroup, parentNode, fileUpdateMonitor, new ArrayList<>(), false); + } + private void setupValidation() { validator = new CompositeValidator(); @@ -405,7 +422,16 @@ public void setValues() { .ifPresent(iconProperty::setValue); parentNode.getGroup().getColor().ifPresent(color -> colorUseProperty.setValue(true)); } - typeExplicitProperty.setValue(true); + if (!selectedEntries.isEmpty()) { + entriesAreSelected.setValue(true); + if (selectedEntries.size() > 1 || useSelectedEntries) { + typeSelectionProperty.setValue(true); + } else { + typeExplicitProperty.setValue(true); + } + } else { + typeExplicitProperty.setValue(true); + } groupHierarchySelectedProperty.setValue(preferencesService.getGroupsPreferences().getDefaultHierarchicalContext()); autoGroupKeywordsOptionProperty.setValue(Boolean.TRUE); } else { diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java index de0c579699b..cddbff20e4f 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java @@ -72,7 +72,11 @@ public class GroupTreeViewModel extends AbstractViewModel { }; private Optional currentDatabase = Optional.empty(); - public GroupTreeViewModel(StateManager stateManager, DialogService dialogService, PreferencesService preferencesService, TaskExecutor taskExecutor, CustomLocalDragboard localDragboard) { + public GroupTreeViewModel(StateManager stateManager, + DialogService dialogService, + PreferencesService preferencesService, + TaskExecutor taskExecutor, + CustomLocalDragboard localDragboard) { this.stateManager = Objects.requireNonNull(stateManager); this.dialogService = Objects.requireNonNull(dialogService); this.preferences = Objects.requireNonNull(preferencesService); @@ -132,14 +136,18 @@ private void onSelectedGroupChanged(ObservableList newValue) /** * Opens "New Group Dialog" and add the resulting group to the root */ - public void addNewGroupToRoot() { + public void addNewGroupToRoot(boolean useSelectedEntries) { if (currentDatabase.isPresent()) { - addNewSubgroup(rootGroup.get(), GroupDialogHeader.GROUP); + addNewSubgroup(rootGroup.get(), GroupDialogHeader.GROUP, useSelectedEntries); } else { dialogService.showWarningDialogAndWait(Localization.lang("Cannot create group"), Localization.lang("Cannot create group. Please create a library first.")); } } + public void addNewGroupToRoot() { + addNewGroupToRoot(false); + } + /** * Gets invoked if the user changes the active database. * We need to get the new group tree and update the view @@ -166,16 +174,23 @@ private void onActiveDatabaseChanged(Optional newDatabase) { currentDatabase = newDatabase; } + public void addNewSubgroup(GroupNodeViewModel parent, GroupDialogHeader groupDialogHeader) { + addNewSubgroup(parent, groupDialogHeader, false); + } + /** * Opens "New Group Dialog" and adds the resulting group as subgroup to the specified group */ - public void addNewSubgroup(GroupNodeViewModel parent, GroupDialogHeader groupDialogHeader) { + public void addNewSubgroup(GroupNodeViewModel parent, GroupDialogHeader groupDialogHeader, boolean useSelectedEntries) { currentDatabase.ifPresent(database -> { Optional newGroup = dialogService.showCustomDialogAndWait(new GroupDialogView( database, parent.getGroupNode(), null, - groupDialogHeader)); + groupDialogHeader, + stateManager.getSelectedEntries().stream().toList(), + useSelectedEntries + )); newGroup.ifPresent(group -> { parent.addSubgroup(group); @@ -260,7 +275,8 @@ public void editGroup(GroupNodeViewModel oldGroup) { database, oldGroup.getGroupNode().getParent().orElse(null), oldGroup.getGroupNode().getGroup(), - GroupDialogHeader.SUBGROUP)); + GroupDialogHeader.SUBGROUP, + stateManager.getSelectedEntries())); newGroup.ifPresent(group -> { AbstractGroup oldGroupDef = oldGroup.getGroupNode().getGroup(); diff --git a/src/main/java/org/jabref/gui/maintable/CreateGroupFromSelectionAction.java b/src/main/java/org/jabref/gui/maintable/CreateGroupFromSelectionAction.java new file mode 100644 index 00000000000..dcff4b25f60 --- /dev/null +++ b/src/main/java/org/jabref/gui/maintable/CreateGroupFromSelectionAction.java @@ -0,0 +1,37 @@ +package org.jabref.gui.maintable; + +import javafx.beans.binding.BooleanExpression; +import org.jabref.gui.DialogService; +import org.jabref.gui.StateManager; +import org.jabref.gui.actions.SimpleCommand; +import org.jabref.gui.groups.GroupDialogView; +import org.jabref.model.entry.field.StandardField; +import org.jabref.model.groups.GroupTreeNode; +import org.jabref.preferences.PreferencesService; + +import static org.jabref.gui.actions.ActionHelper.isFieldSetForSelectedEntry; +import static org.jabref.gui.actions.ActionHelper.needsEntriesSelected; + +public class CreateGroupFromSelectionAction extends SimpleCommand { + + private final DialogService dialogService; + private final StateManager stateManager; + private final PreferencesService preferencesService; + + public CreateGroupFromSelectionAction(DialogService dialogService, StateManager stateManager, PreferencesService preferencesService) { + this.dialogService = dialogService; + this.stateManager = stateManager; + this.preferencesService = preferencesService; + + this.executable.bind(needsEntriesSelected(stateManager)); + + this.setExecutable(stateManager.activeGroupProperty().size() == 1); + } + + @Override + public void execute() { + GroupTreeNode active = stateManager.activeGroupProperty().getFirst(); + active.addSubgroup() + } + +} diff --git a/src/main/java/org/jabref/gui/maintable/RightClickMenu.java b/src/main/java/org/jabref/gui/maintable/RightClickMenu.java index fdd2defec32..07c7b2723db 100644 --- a/src/main/java/org/jabref/gui/maintable/RightClickMenu.java +++ b/src/main/java/org/jabref/gui/maintable/RightClickMenu.java @@ -18,6 +18,7 @@ import org.jabref.gui.exporter.ExportToClipboardAction; import org.jabref.gui.frame.SendAsKindleEmailAction; import org.jabref.gui.frame.SendAsStandardEmailAction; +import org.jabref.gui.groups.GroupDialogView; import org.jabref.gui.keyboard.KeyBindingRepository; import org.jabref.gui.linkedfile.AttachFileAction; import org.jabref.gui.linkedfile.AttachFileFromURLAction; @@ -86,6 +87,7 @@ public static ContextMenu create(BibEntryTableViewModel entry, factory.createMenuItem(StandardActions.OPEN_URL, new OpenUrlAction(dialogService, stateManager, preferencesService)), factory.createMenuItem(StandardActions.SEARCH_SHORTSCIENCE, new SearchShortScienceAction(dialogService, stateManager, preferencesService)), + factory.createMenuItem(StandardActions.CREATE_GROUP_FROM_SELECTION, new CreateGroupFromSelectionAction(stateManager)) new SeparatorMenuItem(), From faca154f72abbf9e074945b0d66efa1e02eae96d Mon Sep 17 00:00:00 2001 From: m-peeler Date: Sat, 6 Jul 2024 21:47:34 -0400 Subject: [PATCH 06/24] Worked to implement right-click-to-make-group functionality Attempted to implement right-click-to-add functionality to the codebase. This ran into several problems and a few Gordian knots which I was attempting, and failing, to untie. This work included using a standardized SimpleCommand to launch the GroupDialogView each time, and accessed the parameters for that through the StateManager. To ensuring this was doable, however, required changing StateManager from tracking GroupNodeViewModels to tracking GroupTreeNodes. Something in this process broke the way that re-renders occur, and now I am facing persistent issues of: - Group numbers not updating when groups are added - Colors on entries not accurately reflecting the groups they are part of --- src/main/java/module-info.java | 1 + .../java/org/jabref/gui/StateManager.java | 1 - .../jabref/gui/actions/StandardActions.java | 1 + .../jabref/gui/groups/GroupDialogView.java | 8 +- .../jabref/gui/groups/GroupNodeViewModel.java | 5 +- .../org/jabref/gui/groups/GroupTreeView.java | 90 ++-- .../jabref/gui/groups/GroupTreeViewModel.java | 403 +++++------------- .../gui/maintable/CreateGroupAction.java | 268 ++++++++++++ .../CreateGroupFromSelectionAction.java | 37 -- .../PreferredGroupAdditionLocation.java | 7 + .../jabref/gui/maintable/RightClickMenu.java | 11 +- src/main/java/org/jabref/model/TreeNode.java | 1 + .../jabref/model/groups/GroupTreeNode.java | 4 +- .../gui/groups/GroupTreeViewModelTest.java | 4 +- 14 files changed, 450 insertions(+), 391 deletions(-) create mode 100644 src/main/java/org/jabref/gui/maintable/CreateGroupAction.java delete mode 100644 src/main/java/org/jabref/gui/maintable/CreateGroupFromSelectionAction.java create mode 100644 src/main/java/org/jabref/gui/maintable/PreferredGroupAdditionLocation.java diff --git a/src/main/java/module-info.java b/src/main/java/module-info.java index e2ff716e3e7..3e10d8dc7e9 100644 --- a/src/main/java/module-info.java +++ b/src/main/java/module-info.java @@ -150,4 +150,5 @@ requires dd.plist; requires mslinks; requires org.apache.httpcomponents.core5.httpcore5; + requires commons.beanutils; } diff --git a/src/main/java/org/jabref/gui/StateManager.java b/src/main/java/org/jabref/gui/StateManager.java index 283702eec08..f2d2b955f80 100644 --- a/src/main/java/org/jabref/gui/StateManager.java +++ b/src/main/java/org/jabref/gui/StateManager.java @@ -140,7 +140,6 @@ public void setSelectedEntries(List newSelectedEntries) { public void setSelectedGroups(BibDatabaseContext database, List newSelectedGroups) { Objects.requireNonNull(newSelectedGroups); - selectedGroups.clear(); selectedGroups.put(database.getUid(), FXCollections.observableArrayList(newSelectedGroups)); } diff --git a/src/main/java/org/jabref/gui/actions/StandardActions.java b/src/main/java/org/jabref/gui/actions/StandardActions.java index cd9a3a0c3f7..e7deb3abe38 100644 --- a/src/main/java/org/jabref/gui/actions/StandardActions.java +++ b/src/main/java/org/jabref/gui/actions/StandardActions.java @@ -37,6 +37,7 @@ public enum StandardActions implements Action { OPEN_URL(Localization.lang("Open URL or DOI"), IconTheme.JabRefIcons.WWW, KeyBinding.OPEN_URL_OR_DOI), SEARCH_SHORTSCIENCE(Localization.lang("Search ShortScience")), CREATE_GROUP_FROM_SELECTION(Localization.lang("Create group from selection")), + CREATE_SUBGROUP_FROM_SELECTION(Localization.lang("Create subgroup from selection")), MERGE_WITH_FETCHED_ENTRY(Localization.lang("Get bibliographic data from %0", "DOI/ISBN/...")), ATTACH_FILE(Localization.lang("Attach file"), IconTheme.JabRefIcons.ATTACH_FILE), ATTACH_FILE_FROM_URL(Localization.lang("Attach file from URL"), IconTheme.JabRefIcons.DOWNLOAD_FILE), diff --git a/src/main/java/org/jabref/gui/groups/GroupDialogView.java b/src/main/java/org/jabref/gui/groups/GroupDialogView.java index 4aea80223c2..f7c1c398b6d 100644 --- a/src/main/java/org/jabref/gui/groups/GroupDialogView.java +++ b/src/main/java/org/jabref/gui/groups/GroupDialogView.java @@ -112,7 +112,7 @@ public class GroupDialogView extends BaseDialog { private final @Nullable GroupTreeNode parentNode; private final @Nullable AbstractGroup editedGroup; private final List selectedEntries; - private final boolean useSelectedEntries; + private final boolean preferUseSelection; private GroupDialogViewModel viewModel; @@ -133,12 +133,12 @@ public GroupDialogView(BibDatabaseContext currentDatabase, @Nullable AbstractGroup editedGroup, GroupDialogHeader groupDialogHeader, List selectedEntries, - boolean useSelectedEntries) { + boolean preferUseSelection) { this.currentDatabase = currentDatabase; this.parentNode = parentNode; this.editedGroup = editedGroup; this.selectedEntries = selectedEntries; - this.useSelectedEntries = useSelectedEntries; + this.preferUseSelection = preferUseSelection; ViewLoader.view(this) .load() @@ -188,7 +188,7 @@ public GroupDialogView(BibDatabaseContext currentDatabase, @FXML public void initialize() { - viewModel = new GroupDialogViewModel(dialogService, currentDatabase, preferencesService, editedGroup, parentNode, fileUpdateMonitor, selectedEntries, useSelectedEntries); + viewModel = new GroupDialogViewModel(dialogService, currentDatabase, preferencesService, editedGroup, parentNode, fileUpdateMonitor, selectedEntries, preferUseSelection); setResultConverter(viewModel::resultConverter); diff --git a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java index 75ce49baf99..875d6ad4075 100644 --- a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java @@ -93,7 +93,10 @@ public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager state } hasChildren = new SimpleBooleanProperty(); hasChildren.bind(Bindings.isNotEmpty(children)); + EasyBind.subscribe(preferencesService.getGroupsPreferences().displayGroupCountProperty(), shouldDisplay -> updateMatchedEntries()); + stateManager.activeGroupProperty().subscribe(this::updateMatchedEntries); + expandedProperty.set(groupNode.getGroup().isExpanded()); expandedProperty.addListener((observable, oldValue, newValue) -> groupNode.getGroup().setExpanded(newValue)); @@ -389,7 +392,7 @@ public boolean canAddEntriesIn() { return true; } else if (group instanceof LastNameGroup || group instanceof RegexKeywordGroup) { return groupNode.getParent() - .map(parent -> parent.getGroup()) + .map(GroupTreeNode::getGroup) .map(groupParent -> groupParent instanceof AutomaticKeywordGroup || groupParent instanceof AutomaticPersonsGroup) .orElse(false); } else if (group instanceof KeywordGroup) { diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeView.java b/src/main/java/org/jabref/gui/groups/GroupTreeView.java index a3c2e74995f..dd352998e67 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeView.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeView.java @@ -4,14 +4,9 @@ import java.lang.reflect.Method; import java.text.DecimalFormat; import java.time.Duration; -import java.util.ArrayList; -import java.util.LinkedList; -import java.util.List; -import java.util.Objects; -import java.util.Optional; +import java.util.*; import java.util.stream.Collectors; -import javafx.application.Platform; import javafx.beans.binding.Bindings; import javafx.beans.property.ObjectProperty; import javafx.css.PseudoClass; @@ -58,8 +53,10 @@ import org.jabref.gui.util.ViewModelTreeTableCellFactory; import org.jabref.gui.util.ViewModelTreeTableRowFactory; import org.jabref.logic.l10n.Localization; +import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; import org.jabref.model.groups.AllEntriesGroup; +import org.jabref.model.groups.GroupTreeNode; import org.jabref.preferences.PreferencesService; import com.tobiasdiez.easybind.EasyBind; @@ -162,6 +159,8 @@ private void createNodes() { this.setBottom(groupBar); } + + private void initialize() { this.localDragboard = stateManager.getLocalDragboard(); viewModel = new GroupTreeViewModel(stateManager, dialogService, preferencesService, taskExecutor, localDragboard); @@ -170,17 +169,18 @@ private void initialize() { groupTree.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE); dragExpansionHandler = new DragExpansionHandler(); - // Set-up bindings - Platform.runLater(() -> - BindingsHelper.bindContentBidirectional( - groupTree.getSelectionModel().getSelectedItems(), - viewModel.selectedGroupsProperty(), - newSelectedGroups -> { - groupTree.getSelectionModel().clearSelection(); - newSelectedGroups.forEach(this::selectNode); - }, - this::updateSelection - )); + BindingsHelper.bindContentBidirectional( + groupTree.getSelectionModel().getSelectedItems(), + viewModel.selectedGroupsProperty(), + newSelectedGroups -> { + groupTree.getSelectionModel().clearSelection(); + newSelectedGroups.forEach(this::selectNode); + }, + model -> { + if (stateManager.getActiveDatabase().isPresent()) { + updateSelection(model.stream().map(node -> node.getValue().getGroupNode()).collect(Collectors.toList()), stateManager.getActiveDatabase().get()); + } + }); // We try to prevent publishing changes in the search field directly to the search task that takes some time // for larger group structures. @@ -229,7 +229,7 @@ private void initialize() { .withContextMenu(this::createContextMenuForGroup) .withEventFilter(MouseEvent.MOUSE_PRESSED, (row, event) -> { if (((MouseEvent) event).getButton() == MouseButton.SECONDARY && !stateManager.getSelectedEntries().isEmpty()) { - // Prevent right-click to select group whe we have selected entries + // Prevent right-click to select group when we have selected entries event.consume(); } else if (event.getTarget() instanceof StackPane pane) { if (pane.getStyleClass().contains("arrow") || pane.getStyleClass().contains("tree-disclosure-node")) { @@ -281,19 +281,7 @@ private StackPane createNumberCell(GroupNodeViewModel group) { } Text text = new Text(); EasyBind.subscribe(preferencesService.getGroupsPreferences().displayGroupCountProperty(), - shouldDisplayGroupCount -> { - if (text.textProperty().isBound()) { - text.textProperty().unbind(); - text.setText(""); - } - - if (shouldDisplayGroupCount) { - text.textProperty().bind(group.getHits().map(Number::intValue).map(this::getFormattedNumber)); - Tooltip tooltip = new Tooltip(); - tooltip.textProperty().bind(group.getHits().asString()); - Tooltip.install(text, tooltip); - } - }); + shouldDisplayGroupCount -> refreshDisplayGroupCountText(text, group)); text.getStyleClass().setAll("text"); text.styleProperty().bind(Bindings.createStringBinding(() -> { @@ -317,6 +305,20 @@ private StackPane createNumberCell(GroupNodeViewModel group) { return node; } + private void refreshDisplayGroupCountText(Text text, GroupNodeViewModel group) { + if (text.textProperty().isBound()) { + text.textProperty().unbind(); + text.setText(""); + } + + if (preferencesService.getGroupsPreferences().displayGroupCountProperty().getValue()) { + text.textProperty().bind(group.getHits().map(Number::intValue).map(this::getFormattedNumber)); + Tooltip tooltip = new Tooltip(); + tooltip.textProperty().bind(group.getHits().asString()); + Tooltip.install(text, tooltip); + } + } + private void handleOnDragExited(TreeTableRow row, GroupNodeViewModel fieldViewModel, DragEvent dragEvent) { ControlHelper.removeDroppingPseudoClasses(row); } @@ -361,7 +363,7 @@ private void handleOnDragDropped(TreeTableRow row, GroupNode } } groupTree.getSelectionModel().clearSelection(); - changedGroups.forEach(value -> selectNode(value, true)); + changedGroups.forEach(value -> selectNode(value.getGroupNode(), true)); if (success) { viewModel.writeGroupChangesToMetaData(); } @@ -392,20 +394,20 @@ private void handleOnDragOver(TreeTableRow row, GroupNodeVie } } - private void updateSelection(List> newSelectedGroups) { + private void updateSelection(List newSelectedGroups, BibDatabaseContext database) { if ((newSelectedGroups == null) || newSelectedGroups.isEmpty()) { - viewModel.selectedGroupsProperty().clear(); + stateManager.getSelectedGroups(database).clear(); } else { - List list = newSelectedGroups.stream().filter(model -> (model != null) && !(model.getValue().getGroupNode().getGroup() instanceof AllEntriesGroup)).map(TreeItem::getValue).collect(Collectors.toList()); - viewModel.selectedGroupsProperty().setAll(list); + List list = newSelectedGroups.stream().filter(model -> (model != null) && !(model.getGroup() instanceof AllEntriesGroup)).collect(Collectors.toList()); + stateManager.getSelectedGroups(database).setAll(list); } } - private void selectNode(GroupNodeViewModel value) { - selectNode(value, true); + private void selectNode(GroupTreeNode value) { + selectNode(value, false); } - private void selectNode(GroupNodeViewModel value, boolean expandParents) { + private void selectNode(GroupTreeNode value, boolean expandParents) { getTreeItemByValue(value) .ifPresent(treeItem -> { if (expandParents) { @@ -419,17 +421,17 @@ private void selectNode(GroupNodeViewModel value, boolean expandParents) { }); } - private Optional> getTreeItemByValue(GroupNodeViewModel value) { + private Optional> getTreeItemByValue(GroupTreeNode value) { return getTreeItemByValue(groupTree.getRoot(), value); } private Optional> getTreeItemByValue(TreeItem root, - GroupNodeViewModel value) { + GroupTreeNode value) { if (root == null) { return Optional.empty(); } - if (root.getValue().equals(value)) { + if (root.getValue().getGroupNode().equals(value)) { return Optional.of(root); } @@ -570,7 +572,7 @@ private ContextMenu createContextMenuForGroup(GroupNodeViewModel group) { } private void addNewGroup() { - viewModel.addNewGroupToRoot(); + viewModel.addNewGroupToRoot(false); } private String getFormattedNumber(int hits) { @@ -661,7 +663,7 @@ public void execute() { case GROUP_REMOVE_WITH_SUBGROUPS -> viewModel.removeGroupAndSubgroups(group); case GROUP_EDIT -> { - viewModel.editGroup(group); + viewModel.editGroup(group.getGroupNode()); groupTree.refresh(); } case GROUP_SUBGROUP_ADD -> diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java index 96eb314522d..2c9c257255b 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java @@ -7,8 +7,8 @@ import java.util.Objects; import java.util.Optional; import java.util.function.Predicate; -import java.util.stream.Collectors; +import com.tobiasdiez.easybind.Subscription; import javafx.beans.property.ListProperty; import javafx.beans.property.ObjectProperty; import javafx.beans.property.SimpleListProperty; @@ -16,29 +16,21 @@ import javafx.beans.property.SimpleStringProperty; import javafx.beans.property.StringProperty; import javafx.collections.FXCollections; +import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; -import javafx.scene.control.Alert; -import javafx.scene.control.ButtonBar; -import javafx.scene.control.ButtonType; import org.jabref.gui.AbstractViewModel; import org.jabref.gui.DialogService; import org.jabref.gui.StateManager; +import org.jabref.gui.maintable.CreateGroupAction; +import org.jabref.gui.maintable.PreferredGroupAdditionLocation; import org.jabref.gui.util.CustomLocalDragboard; import org.jabref.gui.util.TaskExecutor; import org.jabref.logic.l10n.Localization; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; -import org.jabref.model.groups.AbstractGroup; -import org.jabref.model.groups.AutomaticKeywordGroup; -import org.jabref.model.groups.AutomaticPersonsGroup; import org.jabref.model.groups.ExplicitGroup; import org.jabref.model.groups.GroupTreeNode; -import org.jabref.model.groups.RegexKeywordGroup; -import org.jabref.model.groups.SearchGroup; -import org.jabref.model.groups.TexGroup; -import org.jabref.model.groups.WordKeywordGroup; -import org.jabref.model.metadata.MetaData; import org.jabref.preferences.PreferencesService; import com.tobiasdiez.easybind.EasyBind; @@ -46,7 +38,7 @@ public class GroupTreeViewModel extends AbstractViewModel { private final ObjectProperty rootGroup = new SimpleObjectProperty<>(); - private final ListProperty selectedGroups = new SimpleListProperty<>(FXCollections.observableArrayList()); + private final ListProperty selectedGroups = new SimpleListProperty<>(FXCollections.observableArrayList()); private final StateManager stateManager; private final DialogService dialogService; private final PreferencesService preferences; @@ -61,16 +53,24 @@ public class GroupTreeViewModel extends AbstractViewModel { .getName() .compareToIgnoreCase(v1.getName()); private final Comparator compEntries = (GroupTreeNode v1, GroupTreeNode v2) -> { - int numChildren1 = v1.getEntriesInGroup(this.currentDatabase.get().getEntries()).size(); - int numChildren2 = v2.getEntriesInGroup(this.currentDatabase.get().getEntries()).size(); - return Integer.compare(numChildren2, numChildren1); + if (this.currentDatabase.isPresent()) { + int numChildren1 = v1.getEntriesInGroup(this.currentDatabase.get().getEntries()).size(); + int numChildren2 = v2.getEntriesInGroup(this.currentDatabase.get().getEntries()).size(); + return Integer.compare(numChildren2, numChildren1); + } + return 0; }; private final Comparator compEntriesReverse = (GroupTreeNode v1, GroupTreeNode v2) -> { - int numChildren1 = v1.getEntriesInGroup(this.currentDatabase.get().getEntries()).size(); - int numChildren2 = v2.getEntriesInGroup(this.currentDatabase.get().getEntries()).size(); - return Integer.compare(numChildren1, numChildren2); + if (this.currentDatabase.isPresent()) { + int numChildren1 = v1.getEntriesInGroup(this.currentDatabase.get().getEntries()).size(); + int numChildren2 = v2.getEntriesInGroup(this.currentDatabase.get().getEntries()).size(); + return Integer.compare(numChildren1, numChildren2); + } + return 0; }; private Optional currentDatabase = Optional.empty(); + private Optional unsubscribe = Optional.empty(); + public GroupTreeViewModel(StateManager stateManager, DialogService dialogService, @@ -84,25 +84,60 @@ public GroupTreeViewModel(StateManager stateManager, this.localDragboard = Objects.requireNonNull(localDragboard); // Register listener - EasyBind.subscribe(stateManager.activeDatabaseProperty(), this::onActiveDatabaseChanged); - EasyBind.subscribe(selectedGroups, this::onSelectedGroupChanged); + EasyBind.subscribe( + stateManager.activeDatabaseProperty(), + val -> this.onActiveDatabaseChanged() + ); // Set-up bindings filterPredicate.bind(EasyBind.map(filterText, text -> group -> group.isMatchedBy(text))); // Init - refresh(); + onActiveDatabaseChanged(); } - private void refresh() { - onActiveDatabaseChanged(stateManager.activeDatabaseProperty().getValue()); + private void subscribeToSelectedGroups() { + unsubscribe.ifPresent(Subscription::unsubscribe); + if (currentDatabase.isEmpty()) return; + ObservableList list = stateManager.getSelectedGroups(currentDatabase.get()); + list.addListener(this::onSelectedGroupChanged); + unsubscribe = Optional.of(() -> list.removeListener(this::onSelectedGroupChanged)); + } + + /** + * Gets invoked if the user selects a different group. + * We need to notify the {@link StateManager} about this change so that the main table gets updated. + */ + private void onSelectedGroupChanged(ListChangeListener.Change newValue) { + if (!currentDatabase.equals(stateManager.activeDatabaseProperty().getValue())) { + // Active database is not this one: do nothing + return; + } + + currentDatabase.ifPresent(database -> { + if ((newValue == null) || (newValue.getList().isEmpty())) { + selectedGroups.clear(); + } else { + selectedGroups.setAll(newValue.getList()); + } + }); + } + + private void onActiveDatabaseChanged() { + Optional newDatabase = stateManager.activeDatabaseProperty().getValue(); + currentDatabase = newDatabase; + newDatabase.ifPresentOrElse( + this::onActiveDatabaseExists, + this::onActiveDatabaseNull + ); + subscribeToSelectedGroups(); } public ObjectProperty rootGroupProperty() { return rootGroup; } - public ListProperty selectedGroupsProperty() { + public ListProperty selectedGroupsProperty() { return selectedGroups; } @@ -114,64 +149,41 @@ public StringProperty filterTextProperty() { return filterText; } - /** - * Gets invoked if the user selects a different group. - * We need to notify the {@link StateManager} about this change so that the main table gets updated. - */ - private void onSelectedGroupChanged(ObservableList newValue) { - if (!currentDatabase.equals(stateManager.activeDatabaseProperty().getValue())) { - // Switch of database occurred -> do nothing - return; - } - - currentDatabase.ifPresent(database -> { - if ((newValue == null) || newValue.isEmpty()) { - stateManager.clearSelectedGroups(database); - } else { - stateManager.setSelectedGroups(database, newValue.stream().map(GroupNodeViewModel::getGroupNode).collect(Collectors.toList())); - } - }); - } - /** * Opens "New Group Dialog" and add the resulting group to the root */ - public void addNewGroupToRoot(boolean useSelectedEntries) { - if (currentDatabase.isPresent()) { - addNewSubgroup(rootGroup.get(), GroupDialogHeader.GROUP, useSelectedEntries); - } else { - dialogService.showWarningDialogAndWait(Localization.lang("Cannot create group"), Localization.lang("Cannot create group. Please create a library first.")); - } + public void addNewGroupToRoot(boolean preferSelectedEntries) { + CreateGroupAction groupMaker = new CreateGroupAction( + dialogService, + stateManager, + preferSelectedEntries, + PreferredGroupAdditionLocation.ADD_TO_ROOT, + null + ); + groupMaker.execute(); } - public void addNewGroupToRoot() { - addNewGroupToRoot(false); - } /** * Gets invoked if the user changes the active database. * We need to get the new group tree and update the view */ - private void onActiveDatabaseChanged(Optional newDatabase) { - if (newDatabase.isPresent()) { - GroupNodeViewModel newRoot = newDatabase - .map(BibDatabaseContext::getMetaData) - .flatMap(MetaData::getGroups) - .map(root -> new GroupNodeViewModel(newDatabase.get(), stateManager, taskExecutor, root, localDragboard, preferences)) - .orElse(GroupNodeViewModel.getAllEntriesGroup(newDatabase.get(), stateManager, taskExecutor, localDragboard, preferences)); - - rootGroup.setValue(newRoot); - if (stateManager.getSelectedGroups(newDatabase.get()).isEmpty()) { - stateManager.setSelectedGroups(newDatabase.get(), Collections.singletonList(newRoot.getGroupNode())); - } - selectedGroups.setAll( - stateManager.getSelectedGroups(newDatabase.get()).stream() - .map(selectedGroup -> new GroupNodeViewModel(newDatabase.get(), stateManager, taskExecutor, selectedGroup, localDragboard, preferences)) - .collect(Collectors.toList())); - } else { - rootGroup.setValue(null); + private void onActiveDatabaseExists(BibDatabaseContext newDatabase) { + + GroupNodeViewModel newRoot = newDatabase.getMetaData().getGroups() + .map(root -> new GroupNodeViewModel(newDatabase, stateManager, taskExecutor, root, localDragboard, preferences)) + .orElse(GroupNodeViewModel.getAllEntriesGroup(newDatabase, stateManager, taskExecutor, localDragboard, preferences)); + + rootGroup.setValue(newRoot); + if (stateManager.getSelectedGroups(newDatabase).isEmpty()) { + stateManager.setSelectedGroups(newDatabase, Collections.singletonList(newRoot.getGroupNode())); } - currentDatabase = newDatabase; + selectedGroups.setAll( + stateManager.getSelectedGroups(newDatabase)); + } + + private void onActiveDatabaseNull() { + rootGroup.setValue(null); } public void addNewSubgroup(GroupNodeViewModel parent, GroupDialogHeader groupDialogHeader) { @@ -181,224 +193,21 @@ public void addNewSubgroup(GroupNodeViewModel parent, GroupDialogHeader groupDia /** * Opens "New Group Dialog" and adds the resulting group as subgroup to the specified group */ - public void addNewSubgroup(GroupNodeViewModel parent, GroupDialogHeader groupDialogHeader, boolean useSelectedEntries) { - currentDatabase.ifPresent(database -> { - Optional newGroup = dialogService.showCustomDialogAndWait(new GroupDialogView( - database, - parent.getGroupNode(), - null, - groupDialogHeader, - stateManager.getSelectedEntries().stream().toList(), - useSelectedEntries - )); - - newGroup.ifPresent(group -> { - GroupTreeNode newSubgroup = parent.addSubgroup(group); - selectedGroups.setAll(new GroupNodeViewModel(database, stateManager, taskExecutor, newSubgroup, localDragboard, preferences)); - - // TODO: Add undo - // UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(parent, new GroupTreeNodeViewModel(newGroupNode), UndoableAddOrRemoveGroup.ADD_NODE); - // panel.getUndoManager().addEdit(undo); - - // TODO: Expand parent to make new group visible - // parent.expand(); - dialogService.notify(Localization.lang("Added group \"%0\".", group.getName())); - writeGroupChangesToMetaData(); - }); - }); + public void addNewSubgroup(GroupNodeViewModel parent, GroupDialogHeader groupDialogHeader, boolean preferUseSelection) { + CreateGroupAction groupMaker = new CreateGroupAction(dialogService, stateManager); + groupMaker.execute(); } public void writeGroupChangesToMetaData() { currentDatabase.ifPresent(database -> database.getMetaData().setGroups(rootGroup.get().getGroupNode())); } - private boolean isGroupTypeEqual(AbstractGroup oldGroup, AbstractGroup newGroup) { - return oldGroup.getClass().equals(newGroup.getClass()); - } - - /** - * Check if it is necessary to show a group modified, reassign entry dialog
- * Group name change is handled separately - * - * @param oldGroup Original Group - * @param newGroup Edited group - * @return true if just trivial modifications (e.g. color or description) or the relevant group properties are equal, false otherwise - */ - boolean onlyMinorChanges(AbstractGroup oldGroup, AbstractGroup newGroup) { - // we need to use getclass here because we have different subclass inheritance e.g. ExplicitGroup is a subclass of WordKeyWordGroup - if (oldGroup.getClass() == WordKeywordGroup.class) { - WordKeywordGroup oldWordKeywordGroup = (WordKeywordGroup) oldGroup; - WordKeywordGroup newWordKeywordGroup = (WordKeywordGroup) newGroup; - - return Objects.equals(oldWordKeywordGroup.getSearchField().getName(), newWordKeywordGroup.getSearchField().getName()) - && Objects.equals(oldWordKeywordGroup.getSearchExpression(), newWordKeywordGroup.getSearchExpression()) - && Objects.equals(oldWordKeywordGroup.isCaseSensitive(), newWordKeywordGroup.isCaseSensitive()); - } else if (oldGroup.getClass() == RegexKeywordGroup.class) { - RegexKeywordGroup oldRegexKeywordGroup = (RegexKeywordGroup) oldGroup; - RegexKeywordGroup newRegexKeywordGroup = (RegexKeywordGroup) newGroup; - - return Objects.equals(oldRegexKeywordGroup.getSearchField().getName(), newRegexKeywordGroup.getSearchField().getName()) - && Objects.equals(oldRegexKeywordGroup.getSearchExpression(), newRegexKeywordGroup.getSearchExpression()) - && Objects.equals(oldRegexKeywordGroup.isCaseSensitive(), newRegexKeywordGroup.isCaseSensitive()); - } else if (oldGroup.getClass() == SearchGroup.class) { - SearchGroup oldSearchGroup = (SearchGroup) oldGroup; - SearchGroup newSearchGroup = (SearchGroup) newGroup; - - return Objects.equals(oldSearchGroup.getSearchExpression(), newSearchGroup.getSearchExpression()) - && Objects.equals(oldSearchGroup.getSearchFlags(), newSearchGroup.getSearchFlags()); - } else if (oldGroup.getClass() == AutomaticKeywordGroup.class) { - AutomaticKeywordGroup oldAutomaticKeywordGroup = (AutomaticKeywordGroup) oldGroup; - AutomaticKeywordGroup newAutomaticKeywordGroup = (AutomaticKeywordGroup) oldGroup; - - return Objects.equals(oldAutomaticKeywordGroup.getKeywordDelimiter(), newAutomaticKeywordGroup.getKeywordDelimiter()) - && Objects.equals(oldAutomaticKeywordGroup.getKeywordHierarchicalDelimiter(), newAutomaticKeywordGroup.getKeywordHierarchicalDelimiter()) - && Objects.equals(oldAutomaticKeywordGroup.getField().getName(), newAutomaticKeywordGroup.getField().getName()); - } else if (oldGroup.getClass() == AutomaticPersonsGroup.class) { - AutomaticPersonsGroup oldAutomaticPersonsGroup = (AutomaticPersonsGroup) oldGroup; - AutomaticPersonsGroup newAutomaticPersonsGroup = (AutomaticPersonsGroup) newGroup; - - return Objects.equals(oldAutomaticPersonsGroup.getField().getName(), newAutomaticPersonsGroup.getField().getName()); - } else if (oldGroup.getClass() == TexGroup.class) { - TexGroup oldTexGroup = (TexGroup) oldGroup; - TexGroup newTexGroup = (TexGroup) newGroup; - return Objects.equals(oldTexGroup.getFilePath().toString(), newTexGroup.getFilePath().toString()); - } - return true; - } - /** * Opens "Edit Group Dialog" and changes the given group to the edited one. */ - public void editGroup(GroupNodeViewModel oldGroup) { - currentDatabase.ifPresent(database -> { - Optional newGroup = dialogService.showCustomDialogAndWait(new GroupDialogView( - database, - oldGroup.getGroupNode().getParent().orElse(null), - oldGroup.getGroupNode().getGroup(), - GroupDialogHeader.SUBGROUP, - stateManager.getSelectedEntries() - )); - newGroup.ifPresent(group -> { - - AbstractGroup oldGroupDef = oldGroup.getGroupNode().getGroup(); - String oldGroupName = oldGroupDef.getName(); - - boolean groupTypeEqual = isGroupTypeEqual(oldGroupDef, group); - boolean onlyMinorModifications = groupTypeEqual && onlyMinorChanges(oldGroupDef, group); - - // dialog already warns us about this if the new group is named like another existing group - // We need to check if only the name changed as this is relevant for the entry's group field - if (groupTypeEqual && !group.getName().equals(oldGroupName) && onlyMinorModifications) { - int groupsWithSameName = 0; - Optional databaseRootGroup = currentDatabase.get().getMetaData().getGroups(); - if (databaseRootGroup.isPresent()) { - // we need to check the old name for duplicates. If the new group name occurs more than once, it won't matter - groupsWithSameName = databaseRootGroup.get().findChildrenSatisfying(g -> g.getName().equals(oldGroupName)).size(); - } - boolean removePreviousAssignments = true; - // We found more than 2 groups, so we cannot simply remove old assignment - if (groupsWithSameName >= 2) { - removePreviousAssignments = false; - } - - oldGroup.getGroupNode().setGroup( - group, - true, - removePreviousAssignments, - database.getEntries()); - - dialogService.notify(Localization.lang("Modified group \"%0\".", group.getName())); - writeGroupChangesToMetaData(); - // This is ugly but we have no proper update mechanism in place to propagate the changes, so redraw everything - refresh(); - return; - } - - if (groupTypeEqual && onlyMinorChanges(oldGroup.getGroupNode().getGroup(), group)) { - oldGroup.getGroupNode().setGroup( - group, - true, - true, - database.getEntries()); - - writeGroupChangesToMetaData(); - refresh(); - return; - } - - // Major modifications - - String content = Localization.lang("Assign the original group's entries to this group?"); - ButtonType keepAssignments = new ButtonType(Localization.lang("Assign"), ButtonBar.ButtonData.YES); - ButtonType removeAssignments = new ButtonType(Localization.lang("Do not assign"), ButtonBar.ButtonData.NO); - ButtonType cancel = new ButtonType(Localization.lang("Cancel"), ButtonBar.ButtonData.CANCEL_CLOSE); - - if (newGroup.get().getClass() == WordKeywordGroup.class) { - content = content + "\n\n" + - Localization.lang("(Note: If original entries lack keywords to qualify for the new group configuration, confirming here will add them)"); - } - Optional previousAssignments = dialogService.showCustomButtonDialogAndWait(Alert.AlertType.WARNING, - Localization.lang("Change of Grouping Method"), - content, - keepAssignments, - removeAssignments, - cancel); - boolean removePreviousAssignments = (oldGroup.getGroupNode().getGroup() instanceof ExplicitGroup) - && (group instanceof ExplicitGroup); - - int groupsWithSameName = 0; - Optional databaseRootGroup = currentDatabase.get().getMetaData().getGroups(); - if (databaseRootGroup.isPresent()) { - String name = oldGroup.getGroupNode().getGroup().getName(); - groupsWithSameName = databaseRootGroup.get().findChildrenSatisfying(g -> g.getName().equals(name)).size(); - } - // okay we found more than 2 groups with the same name - // If we only found one we can still do it - if (groupsWithSameName >= 2) { - removePreviousAssignments = false; - } - - if (previousAssignments.isPresent() && (previousAssignments.get().getButtonData() == ButtonBar.ButtonData.YES)) { - oldGroup.getGroupNode().setGroup( - group, - true, - removePreviousAssignments, - database.getEntries()); - } else if (previousAssignments.isPresent() && (previousAssignments.get().getButtonData() == ButtonBar.ButtonData.NO)) { - oldGroup.getGroupNode().setGroup( - group, - false, - removePreviousAssignments, - database.getEntries()); - } else if (previousAssignments.isPresent() && (previousAssignments.get().getButtonData() == ButtonBar.ButtonData.CANCEL_CLOSE)) { - return; - } - - // stateManager.getEntriesInCurrentDatabase()); - - // TODO: Add undo - // Store undo information. - // AbstractUndoableEdit undoAddPreviousEntries = null; - // UndoableModifyGroup undo = new UndoableModifyGroup(GroupSelector.this, groupsRoot, node, newGroup); - // if (undoAddPreviousEntries == null) { - // panel.getUndoManager().addEdit(undo); - // } else { - // NamedCompound nc = new NamedCompound("Modify Group"); - // nc.addEdit(undo); - // nc.addEdit(undoAddPreviousEntries); - // nc.end();/ - // panel.getUndoManager().addEdit(nc); - // } - // if (!addChange.isEmpty()) { - // undoAddPreviousEntries = UndoableChangeEntriesOfGroup.getUndoableEdit(null, addChange); - // } - - dialogService.notify(Localization.lang("Modified group \"%0\".", group.getName())); - writeGroupChangesToMetaData(); - // This is ugly but we have no proper update mechanism in place to propagate the changes, so redraw everything - refresh(); - }); - }); + public void editGroup(GroupTreeNode oldGroup) { + CreateGroupAction groupMaker = new CreateGroupAction(dialogService, stateManager, false, PreferredGroupAdditionLocation.ADD_BESIDE, oldGroup); + groupMaker.execute(); } public void removeSubgroups(GroupNodeViewModel group) { @@ -410,7 +219,7 @@ public void removeSubgroups(GroupNodeViewModel group) { // final UndoableModifySubtree undo = new UndoableModifySubtree(getGroupTreeRoot(), node, "Remove subgroups"); // panel.getUndoManager().addEdit(undo); for (GroupNodeViewModel child : group.getChildren()) { - removeGroupsAndSubGroupsFromEntries(child); + removeGroupsAndSubGroupsFromEntries(child.getGroupNode()); } group.getGroupNode().removeAllChildren(); dialogService.notify(Localization.lang("Removed all subgroups of group \"%0\".", group.getDisplayName())); @@ -437,13 +246,11 @@ public void removeGroupKeepSubgroups(GroupNodeViewModel group) { // final UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(groupsRoot, node, UndoableAddOrRemoveGroup.REMOVE_NODE_KEEP_CHILDREN); // panel.getUndoManager().addEdit(undo); - List selectedGroupNodes = new ArrayList<>(selectedGroups); + List selectedGroupNodes = new ArrayList<>(selectedGroups); selectedGroupNodes.forEach(eachNode -> { - GroupTreeNode groupNode = eachNode.getGroupNode(); - - groupNode.getParent() - .ifPresent(parent -> groupNode.moveAllChildrenTo(parent, parent.getIndexOfChild(groupNode).get())); - groupNode.removeFromParent(); + eachNode.getParent() + .ifPresent(parent -> eachNode.moveAllChildrenTo(parent, parent.getIndexOfChild(eachNode).get())); + eachNode.removeFromParent(); }); if (selectedGroupNodes.size() > 1) { @@ -477,10 +284,10 @@ public void removeGroupAndSubgroups(GroupNodeViewModel group) { // final UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(groupsRoot, node, UndoableAddOrRemoveGroup.REMOVE_NODE_AND_CHILDREN); // panel.getUndoManager().addEdit(undo); - ArrayList selectedGroupNodes = new ArrayList<>(selectedGroups); + ArrayList selectedGroupNodes = new ArrayList<>(selectedGroups); selectedGroupNodes.forEach(eachNode -> { removeGroupsAndSubGroupsFromEntries(eachNode); - eachNode.getGroupNode().removeFromParent(); + eachNode.removeFromParent(); }); if (selectedGroupNodes.size() > 1) { @@ -514,10 +321,10 @@ public void removeGroupNoSubgroups(GroupNodeViewModel group) { // final UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(groupsRoot, node, UndoableAddOrRemoveGroup.REMOVE_NODE_WITHOUT_CHILDREN); // panel.getUndoManager().addEdit(undo); - ArrayList selectedGroupNodes = new ArrayList<>(selectedGroups); + ArrayList selectedGroupNodes = new ArrayList<>(selectedGroups); selectedGroupNodes.forEach(eachNode -> { removeGroupsAndSubGroupsFromEntries(eachNode); - eachNode.getGroupNode().removeFromParent(); + eachNode.removeFromParent(); }); if (selectedGroupNodes.size() > 1) { @@ -529,22 +336,22 @@ public void removeGroupNoSubgroups(GroupNodeViewModel group) { } } - void removeGroupsAndSubGroupsFromEntries(GroupNodeViewModel group) { - for (GroupNodeViewModel child : group.getChildren()) { + void removeGroupsAndSubGroupsFromEntries(GroupTreeNode group) { + for (GroupTreeNode child : group.getChildren()) { removeGroupsAndSubGroupsFromEntries(child); } // only remove explicit groups from the entries, keyword groups should not be deleted - if (group.getGroupNode().getGroup() instanceof ExplicitGroup) { + if (group.getGroup() instanceof ExplicitGroup) { int groupsWithSameName = 0; - String name = group.getGroupNode().getGroup().getName(); + String name = group.getGroup().getName(); Optional rootGroup = currentDatabase.get().getMetaData().getGroups(); if (rootGroup.isPresent()) { groupsWithSameName = rootGroup.get().findChildrenSatisfying(g -> g.getName().equals(name)).size(); } if (groupsWithSameName < 2) { - List entriesInGroup = group.getGroupNode().getEntriesInGroup(this.currentDatabase.get().getEntries()); - group.getGroupNode().removeEntriesFromGroup(entriesInGroup); + List entriesInGroup = group.getEntriesInGroup(this.currentDatabase.get().getEntries()); + group.removeEntriesFromGroup(entriesInGroup); } } } diff --git a/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java b/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java new file mode 100644 index 00000000000..980360e9abc --- /dev/null +++ b/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java @@ -0,0 +1,268 @@ +package org.jabref.gui.maintable; + +import javafx.scene.Group; +import javafx.scene.control.Alert; +import javafx.scene.control.ButtonBar; +import javafx.scene.control.ButtonType; +import org.jabref.gui.DialogService; +import org.jabref.gui.StateManager; +import org.jabref.gui.actions.SimpleCommand; +import org.jabref.gui.groups.GroupDialogHeader; +import org.jabref.gui.groups.GroupDialogView; +import org.jabref.logic.l10n.Localization; +import org.jabref.model.database.BibDatabaseContext; +import org.jabref.model.groups.*; +import org.jspecify.annotations.Nullable; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Objects; +import java.util.Optional; + +import static org.jabref.gui.actions.ActionHelper.needsEntriesSelected; + +public class CreateGroupAction extends SimpleCommand { + + private final DialogService dialogService; + private final StateManager stateManager; + private final boolean preferSelectedEntries; + private final PreferredGroupAdditionLocation preferredAdditionLocation; + private final GroupTreeNode editGroup; + + public CreateGroupAction( + DialogService dialogService, + StateManager stateManager, + boolean preferSelectedEntries, + PreferredGroupAdditionLocation preferredAdditionLocation, + @Nullable GroupTreeNode editGroup + ) { + this.dialogService = dialogService; + this.stateManager = stateManager; + this.preferSelectedEntries = preferSelectedEntries; + this.preferredAdditionLocation = preferredAdditionLocation; + this.editGroup = editGroup; + + this.executable.bind(needsEntriesSelected(stateManager)); + } + + public CreateGroupAction( + DialogService dialogService, + StateManager stateManager + ) { + this(dialogService, stateManager, false, PreferredGroupAdditionLocation.ADD_BESIDE, null); + } + + public void writeGroupChangesToMetaData(BibDatabaseContext database, GroupTreeNode newGroup) { + database.getMetaData().setGroups(newGroup.getRoot()); + } + + @Override + public void execute() { + Optional database = stateManager.getActiveDatabase(); + Optional active; + + if (!stateManager.activeGroupProperty().isEmpty()) + active = Optional.of(stateManager.activeGroupProperty().getFirst()); + else if (database.isPresent()) + active = database.get().getMetaData().getGroups(); + else + active = Optional.empty(); + + if (database.isPresent() && active.isPresent()) { + boolean isRoot = active.get().isRoot(); + + final GroupTreeNode groupFallsUnder; + if (editGroup != null && editGroup.getParent().isPresent()) { + groupFallsUnder = editGroup.getParent().get(); + } else if (editGroup != null) { + groupFallsUnder = editGroup; + } else if (preferredAdditionLocation == PreferredGroupAdditionLocation.ADD_TO_ROOT) { + groupFallsUnder = active.get().getRoot(); + } else if (preferredAdditionLocation == PreferredGroupAdditionLocation.ADD_BESIDE + && !isRoot) { + groupFallsUnder = active.get().getParent().get(); + } else { + groupFallsUnder = active.get(); + } + + Optional newGroup = dialogService.showCustomDialogAndWait( + new GroupDialogView( + database.get(), + groupFallsUnder, + editGroup != null ? editGroup.getGroup() : null, + isRoot ? GroupDialogHeader.GROUP : GroupDialogHeader.SUBGROUP, + stateManager.getSelectedEntries(), + preferSelectedEntries + )); + + newGroup.ifPresent(group -> { + if (editGroup != null) { + groupEditActions(editGroup, group, database.get()); + return; + } + GroupTreeNode newSubgroup = groupFallsUnder.addSubgroup(group); + stateManager.getSelectedGroups(database.get()).setAll(newSubgroup); + + // TODO: Add undo + // UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(parent, new GroupTreeNodeViewModel(newGroupNode), UndoableAddOrRemoveGroup.ADD_NODE); + // panel.getUndoManager().addEdit(undo); + + dialogService.notify(Localization.lang("Added group \"%0\".", group.getName())); + writeGroupChangesToMetaData(database.get(), newSubgroup); + }); + } else { + dialogService.showWarningDialogAndWait(Localization.lang("Cannot create group"), Localization.lang("Cannot create group. Please create a library first.")); + } + + } + + private void groupEditActions(GroupTreeNode oldGroup, AbstractGroup newGroup, BibDatabaseContext database) { + AbstractGroup oldGroupDef = oldGroup.getGroup(); + String oldGroupName = oldGroupDef.getName(); + + boolean groupTypeEqual = isGroupTypeEqual(oldGroupDef, newGroup); + boolean onlyMinorModifications = groupTypeEqual && changesAreMinor(oldGroupDef, newGroup); + + // I inherited this from the previous maintainer. Haven't dug into which has better priority, + // what default behavior is, or why this appears replicated -- all my changes are simply refactoring. + boolean keepPreviousAssignments; + boolean removePreviousAssignments; + GroupTreeNode finalResults; + + // dialog already warns us about this if the new group is named like another existing group + // We need to check if only the name changed as this is relevant for the entry's group field + if (groupTypeEqual && !newGroup.getName().equals(oldGroupName) && onlyMinorModifications) { + + keepPreviousAssignments = true; + // If the name is unique, we want to remove the old database with the same name. + // Otherwise, we need to keep them. + removePreviousAssignments = nameIsUnique(oldGroup, newGroup, database); + + } else if (groupTypeEqual && changesAreMinor(oldGroup.getGroup(), newGroup)) { + + keepPreviousAssignments = true; + removePreviousAssignments = true; + + } else { + // Major modifications + Optional reassignmentResponse = showConfirmationPanel(newGroup.getClass() == WordKeywordGroup.class); + if (reassignmentResponse.isEmpty() || reassignmentResponse.get().getButtonData() == ButtonBar.ButtonData.CANCEL_CLOSE) { + return; + } + removePreviousAssignments = (oldGroup.getGroup() instanceof ExplicitGroup) + && (newGroup instanceof ExplicitGroup) + && nameIsUnique(oldGroup, newGroup, database); + keepPreviousAssignments = (reassignmentResponse.get().getButtonData() == ButtonBar.ButtonData.YES); + } + + + // TODO: Add undo + // Store undo information. + // AbstractUndoableEdit undoAddPreviousEntries = null; + // UndoableModifyGroup undo = new UndoableModifyGroup(GroupSelector.this, groupsRoot, node, newGroup); + // if (undoAddPreviousEntries == null) { + // panel.getUndoManager().addEdit(undo); + // } else { + // NamedCompound nc = new NamedCompound("Modify Group"); + // nc.addEdit(undo); + // nc.addEdit(undoAddPreviousEntries); + // nc.end();/ + // panel.getUndoManager().addEdit(nc); + // } + // if (!addChange.isEmpty()) { + // undoAddPreviousEntries = UndoableChangeEntriesOfGroup.getUndoableEdit(null, addChange); + // } + + oldGroup.setGroup( + newGroup, + keepPreviousAssignments, + removePreviousAssignments, + database.getEntries()); + stateManager.getSelectedGroups(database).setAll(oldGroup); + dialogService.notify(Localization.lang("Modified group \"%0\".", oldGroup.getName())); + writeGroupChangesToMetaData(database, oldGroup); + + } + + private boolean isGroupTypeEqual(AbstractGroup oldGroup, AbstractGroup newGroup) { + return oldGroup.getClass().equals(newGroup.getClass()); + } + + /** + * Check if it is necessary to show a group modified, reassign entry dialog
+ * Group name change is handled separately + * + * @param oldGroup Original Group + * @param newGroup Edited group + * @return true if just trivial modifications (e.g. color or description) or the relevant group properties are equal, false otherwise + */ + private boolean changesAreMinor(AbstractGroup oldGroup, AbstractGroup newGroup) { + // we need to use getclass here because we have different subclass inheritance e.g. ExplicitGroup is a subclass of WordKeyWordGroup + if (oldGroup.getClass() == WordKeywordGroup.class) { + WordKeywordGroup oldWordKeywordGroup = (WordKeywordGroup) oldGroup; + WordKeywordGroup newWordKeywordGroup = (WordKeywordGroup) newGroup; + + return Objects.equals(oldWordKeywordGroup.getSearchField().getName(), newWordKeywordGroup.getSearchField().getName()) + && Objects.equals(oldWordKeywordGroup.getSearchExpression(), newWordKeywordGroup.getSearchExpression()) + && Objects.equals(oldWordKeywordGroup.isCaseSensitive(), newWordKeywordGroup.isCaseSensitive()); + } else if (oldGroup.getClass() == RegexKeywordGroup.class) { + RegexKeywordGroup oldRegexKeywordGroup = (RegexKeywordGroup) oldGroup; + RegexKeywordGroup newRegexKeywordGroup = (RegexKeywordGroup) newGroup; + + return Objects.equals(oldRegexKeywordGroup.getSearchField().getName(), newRegexKeywordGroup.getSearchField().getName()) + && Objects.equals(oldRegexKeywordGroup.getSearchExpression(), newRegexKeywordGroup.getSearchExpression()) + && Objects.equals(oldRegexKeywordGroup.isCaseSensitive(), newRegexKeywordGroup.isCaseSensitive()); + } else if (oldGroup.getClass() == SearchGroup.class) { + SearchGroup oldSearchGroup = (SearchGroup) oldGroup; + SearchGroup newSearchGroup = (SearchGroup) newGroup; + + return Objects.equals(oldSearchGroup.getSearchExpression(), newSearchGroup.getSearchExpression()) + && Objects.equals(oldSearchGroup.getSearchFlags(), newSearchGroup.getSearchFlags()); + } else if (oldGroup.getClass() == AutomaticKeywordGroup.class) { + AutomaticKeywordGroup oldAutomaticKeywordGroup = (AutomaticKeywordGroup) oldGroup; + AutomaticKeywordGroup newAutomaticKeywordGroup = (AutomaticKeywordGroup) oldGroup; + + return Objects.equals(oldAutomaticKeywordGroup.getKeywordDelimiter(), newAutomaticKeywordGroup.getKeywordDelimiter()) + && Objects.equals(oldAutomaticKeywordGroup.getKeywordHierarchicalDelimiter(), newAutomaticKeywordGroup.getKeywordHierarchicalDelimiter()) + && Objects.equals(oldAutomaticKeywordGroup.getField().getName(), newAutomaticKeywordGroup.getField().getName()); + } else if (oldGroup.getClass() == AutomaticPersonsGroup.class) { + AutomaticPersonsGroup oldAutomaticPersonsGroup = (AutomaticPersonsGroup) oldGroup; + AutomaticPersonsGroup newAutomaticPersonsGroup = (AutomaticPersonsGroup) newGroup; + + return Objects.equals(oldAutomaticPersonsGroup.getField().getName(), newAutomaticPersonsGroup.getField().getName()); + } else if (oldGroup.getClass() == TexGroup.class) { + TexGroup oldTexGroup = (TexGroup) oldGroup; + TexGroup newTexGroup = (TexGroup) newGroup; + return Objects.equals(oldTexGroup.getFilePath().toString(), newTexGroup.getFilePath().toString()); + } + return true; + } + + private boolean nameIsUnique(GroupTreeNode oldGroup, AbstractGroup newGroup, BibDatabaseContext database) { + int groupsWithSameName = 0; + Optional databaseRootGroup = database.getMetaData().getGroups(); + if (databaseRootGroup.isPresent()) { + // we need to check the old name for duplicates. If the new group name occurs more than once, it won't matter + groupsWithSameName = databaseRootGroup.get().findChildrenSatisfying(g -> g.getName().equals(oldGroup.getName())).size(); + } + return !(groupsWithSameName >= 2); + } + + private Optional showConfirmationPanel(boolean isNowWordKeywordGroup) { + String content = Localization.lang("Assign the original group's entries to this group?"); + ButtonType keepAssignments = new ButtonType(Localization.lang("Assign"), ButtonBar.ButtonData.YES); + ButtonType removeAssignments = new ButtonType(Localization.lang("Do not assign"), ButtonBar.ButtonData.NO); + ButtonType cancel = new ButtonType(Localization.lang("Cancel"), ButtonBar.ButtonData.CANCEL_CLOSE); + + if (isNowWordKeywordGroup) { + content = content + "\n\n" + + Localization.lang("(Note: If original entries lack keywords to qualify for the new group configuration, confirming here will add them)"); + } + return dialogService.showCustomButtonDialogAndWait(Alert.AlertType.WARNING, + Localization.lang("Change of Grouping Method"), + content, + keepAssignments, + removeAssignments, + cancel); + } +} diff --git a/src/main/java/org/jabref/gui/maintable/CreateGroupFromSelectionAction.java b/src/main/java/org/jabref/gui/maintable/CreateGroupFromSelectionAction.java deleted file mode 100644 index dcff4b25f60..00000000000 --- a/src/main/java/org/jabref/gui/maintable/CreateGroupFromSelectionAction.java +++ /dev/null @@ -1,37 +0,0 @@ -package org.jabref.gui.maintable; - -import javafx.beans.binding.BooleanExpression; -import org.jabref.gui.DialogService; -import org.jabref.gui.StateManager; -import org.jabref.gui.actions.SimpleCommand; -import org.jabref.gui.groups.GroupDialogView; -import org.jabref.model.entry.field.StandardField; -import org.jabref.model.groups.GroupTreeNode; -import org.jabref.preferences.PreferencesService; - -import static org.jabref.gui.actions.ActionHelper.isFieldSetForSelectedEntry; -import static org.jabref.gui.actions.ActionHelper.needsEntriesSelected; - -public class CreateGroupFromSelectionAction extends SimpleCommand { - - private final DialogService dialogService; - private final StateManager stateManager; - private final PreferencesService preferencesService; - - public CreateGroupFromSelectionAction(DialogService dialogService, StateManager stateManager, PreferencesService preferencesService) { - this.dialogService = dialogService; - this.stateManager = stateManager; - this.preferencesService = preferencesService; - - this.executable.bind(needsEntriesSelected(stateManager)); - - this.setExecutable(stateManager.activeGroupProperty().size() == 1); - } - - @Override - public void execute() { - GroupTreeNode active = stateManager.activeGroupProperty().getFirst(); - active.addSubgroup() - } - -} diff --git a/src/main/java/org/jabref/gui/maintable/PreferredGroupAdditionLocation.java b/src/main/java/org/jabref/gui/maintable/PreferredGroupAdditionLocation.java new file mode 100644 index 00000000000..9e2ffa4b6b4 --- /dev/null +++ b/src/main/java/org/jabref/gui/maintable/PreferredGroupAdditionLocation.java @@ -0,0 +1,7 @@ +package org.jabref.gui.maintable; + +public enum PreferredGroupAdditionLocation { + ADD_TO_ROOT, + ADD_BESIDE, + ADD_BELOW +} diff --git a/src/main/java/org/jabref/gui/maintable/RightClickMenu.java b/src/main/java/org/jabref/gui/maintable/RightClickMenu.java index 07c7b2723db..f37bd9f2d8f 100644 --- a/src/main/java/org/jabref/gui/maintable/RightClickMenu.java +++ b/src/main/java/org/jabref/gui/maintable/RightClickMenu.java @@ -18,7 +18,6 @@ import org.jabref.gui.exporter.ExportToClipboardAction; import org.jabref.gui.frame.SendAsKindleEmailAction; import org.jabref.gui.frame.SendAsStandardEmailAction; -import org.jabref.gui.groups.GroupDialogView; import org.jabref.gui.keyboard.KeyBindingRepository; import org.jabref.gui.linkedfile.AttachFileAction; import org.jabref.gui.linkedfile.AttachFileFromURLAction; @@ -75,6 +74,15 @@ public static ContextMenu create(BibEntryTableViewModel entry, SpecialFieldMenuItemFactory.getSpecialFieldSingleItem(SpecialField.PRINTED, factory, () -> libraryTab, dialogService, preferencesService, undoManager, stateManager), SpecialFieldMenuItemFactory.createSpecialFieldMenu(SpecialField.PRIORITY, factory, () -> libraryTab, dialogService, preferencesService, undoManager, stateManager), SpecialFieldMenuItemFactory.createSpecialFieldMenu(SpecialField.READ_STATUS, factory, () -> libraryTab, dialogService, preferencesService, undoManager, stateManager), + factory.createMenuItem( + // The new group is placed inside the currently active group, so + // the button's name changes based on whether we are in the root group + // or a subgroup. + stateManager.activeGroupProperty().isEmpty() || stateManager.activeGroupProperty().getFirst().getParent().isEmpty() + ? StandardActions.CREATE_GROUP_FROM_SELECTION + : StandardActions.CREATE_SUBGROUP_FROM_SELECTION, + new CreateGroupAction(dialogService, stateManager, true, PreferredGroupAdditionLocation.ADD_BELOW, null)), + new SeparatorMenuItem(), @@ -87,7 +95,6 @@ public static ContextMenu create(BibEntryTableViewModel entry, factory.createMenuItem(StandardActions.OPEN_URL, new OpenUrlAction(dialogService, stateManager, preferencesService)), factory.createMenuItem(StandardActions.SEARCH_SHORTSCIENCE, new SearchShortScienceAction(dialogService, stateManager, preferencesService)), - factory.createMenuItem(StandardActions.CREATE_GROUP_FROM_SELECTION, new CreateGroupFromSelectionAction(stateManager)) new SeparatorMenuItem(), diff --git a/src/main/java/org/jabref/model/TreeNode.java b/src/main/java/org/jabref/model/TreeNode.java index fcf0b8ab26e..0ba3ea1a204 100644 --- a/src/main/java/org/jabref/model/TreeNode.java +++ b/src/main/java/org/jabref/model/TreeNode.java @@ -10,6 +10,7 @@ import javafx.collections.FXCollections; import javafx.collections.ObservableList; +import org.jabref.model.groups.GroupTreeNode; /** * Represents a node in a tree. diff --git a/src/main/java/org/jabref/model/groups/GroupTreeNode.java b/src/main/java/org/jabref/model/groups/GroupTreeNode.java index d5954181d9d..a80ef1f159a 100644 --- a/src/main/java/org/jabref/model/groups/GroupTreeNode.java +++ b/src/main/java/org/jabref/model/groups/GroupTreeNode.java @@ -140,7 +140,7 @@ public int hashCode() { * * @param entries List of {@link BibEntry} to search for * @param requireAll Whether to return only groups that must contain all entries - * @return List of {@link GroupTreeNode} containing the matches. {@link AllEntriesGroup} is always contained} + * @return List of {@link GroupTreeNode} containing the matches. {@link AllEntriesGroup} is always contained. */ public List getContainingGroups(List entries, boolean requireAll) { List groups = new ArrayList<>(); @@ -255,7 +255,7 @@ public boolean matches(BibEntry entry) { } /** - * Get the path from the root of the tree as a string (every group name is separated by {@link #PATH_DELIMITER}. + * Get the path from the root of the tree as a string every group name is separated by {@link #PATH_DELIMITER}. *

* The name of the root is not included. */ diff --git a/src/test/java/org/jabref/gui/groups/GroupTreeViewModelTest.java b/src/test/java/org/jabref/gui/groups/GroupTreeViewModelTest.java index bdb27ee2fc1..621b7465ec5 100644 --- a/src/test/java/org/jabref/gui/groups/GroupTreeViewModelTest.java +++ b/src/test/java/org/jabref/gui/groups/GroupTreeViewModelTest.java @@ -72,7 +72,7 @@ void explicitGroupsAreRemovedFromEntriesOnDelete() { GroupNodeViewModel model = new GroupNodeViewModel(databaseContext, stateManager, taskExecutor, group, new CustomLocalDragboard(), preferencesService); model.addEntriesToGroup(databaseContext.getEntries()); - groupTree.removeGroupsAndSubGroupsFromEntries(model); + groupTree.removeGroupsAndSubGroupsFromEntries(model.getGroupNode()); assertEquals(Optional.empty(), entry.getField(StandardField.GROUPS)); } @@ -86,7 +86,7 @@ void keywordGroupsAreNotRemovedFromEntriesOnDelete() { GroupNodeViewModel model = new GroupNodeViewModel(databaseContext, stateManager, taskExecutor, group, new CustomLocalDragboard(), preferencesService); model.addEntriesToGroup(databaseContext.getEntries()); - groupTree.removeGroupsAndSubGroupsFromEntries(model); + groupTree.removeGroupsAndSubGroupsFromEntries(model.getGroupNode()); assertEquals(groupName, entry.getField(StandardField.KEYWORDS).get()); } From 3a6037ee3d1841f17743c85c61b7e3d64a2a4fc3 Mon Sep 17 00:00:00 2001 From: m-peeler Date: Tue, 9 Jul 2024 18:21:45 -0400 Subject: [PATCH 07/24] Feature added: Right Click on entries to create group from selection Added functionality to allow users to right click to make a new group from the selection. Updated all group creation and group editing to use the CreateGroupAction class, extending SimpleCommand so it would work with the right-click menu. Getting this functionality extracted into an Action that could be called with the available states stored in stateManager required some pretty large changes and bug fixes related to synchronizing the various lists of active and selected entries. Additionally, fixed a bug where when new entries were dropped into a list, the displayed number of entries did not update. --- .../java/org/jabref/gui/StateManager.java | 4 +- .../jabref/gui/exporter/ExportCommand.java | 3 +- .../jabref/gui/groups/GroupDialogView.java | 14 ++-- .../gui/groups/GroupDialogViewModel.java | 4 +- .../jabref/gui/groups/GroupNodeViewModel.java | 43 ++++++----- .../org/jabref/gui/groups/GroupTreeView.java | 73 ++++++++++++------- .../jabref/gui/groups/GroupTreeViewModel.java | 54 ++++---------- .../gui/maintable/CreateGroupAction.java | 41 +++++------ .../jabref/logic/database/DatabaseMerger.java | 13 ++-- .../model/database/BibDatabaseContext.java | 3 +- .../jabref/model/groups/AbstractGroup.java | 15 +++- .../jabref/model/groups/GroupTreeNode.java | 58 ++++++++------- .../model/groups/RegexKeywordGroup.java | 2 +- .../gui/groups/GroupDialogViewModelTest.java | 6 +- .../gui/groups/GroupTreeViewModelTest.java | 20 ++--- 15 files changed, 185 insertions(+), 168 deletions(-) diff --git a/src/main/java/org/jabref/gui/StateManager.java b/src/main/java/org/jabref/gui/StateManager.java index f2d2b955f80..71fb9f0f96c 100644 --- a/src/main/java/org/jabref/gui/StateManager.java +++ b/src/main/java/org/jabref/gui/StateManager.java @@ -75,7 +75,8 @@ public class StateManager { private final ObservableList searchHistory = FXCollections.observableArrayList(); public StateManager() { - activeGroups.bind(Bindings.valueAt(selectedGroups, activeDatabase.orElseOpt(null).map(BibDatabaseContext::getUid))); + activeGroups.bind(Bindings.valueAt( + selectedGroups, activeDatabase.orElseOpt(null).map(BibDatabaseContext::getUid)).orElse(FXCollections.emptyObservableList())); } public ObservableList getVisibleSidePaneComponents() { @@ -161,6 +162,7 @@ public void setActiveDatabase(BibDatabaseContext database) { LOGGER.info("No open database detected"); activeDatabaseProperty().set(Optional.empty()); } else { + selectedGroups.computeIfAbsent(database.getUid(), k -> FXCollections.emptyObservableList()); activeDatabaseProperty().set(Optional.of(database)); } } diff --git a/src/main/java/org/jabref/gui/exporter/ExportCommand.java b/src/main/java/org/jabref/gui/exporter/ExportCommand.java index ccf81b353ff..64352057c49 100644 --- a/src/main/java/org/jabref/gui/exporter/ExportCommand.java +++ b/src/main/java/org/jabref/gui/exporter/ExportCommand.java @@ -8,6 +8,7 @@ import java.util.function.Supplier; import java.util.stream.Collectors; +import javafx.collections.FXCollections; import javafx.stage.FileChooser; import javafx.util.Duration; @@ -111,7 +112,7 @@ private void export(Path file, FileChooser.ExtensionFilter selectedExtensionFilt // All entries entries = stateManager.getActiveDatabase() .map(BibDatabaseContext::getEntries) - .orElse(Collections.emptyList()); + .orElse(FXCollections.emptyObservableList()); } List fileDirForDatabase = stateManager.getActiveDatabase() diff --git a/src/main/java/org/jabref/gui/groups/GroupDialogView.java b/src/main/java/org/jabref/gui/groups/GroupDialogView.java index f7c1c398b6d..b8db49eade0 100644 --- a/src/main/java/org/jabref/gui/groups/GroupDialogView.java +++ b/src/main/java/org/jabref/gui/groups/GroupDialogView.java @@ -265,7 +265,7 @@ public void initialize() { validationVisualizer.initVisualization(viewModel.keywordRegexValidationStatus(), keywordGroupSearchTerm); validationVisualizer.initVisualization(viewModel.keywordSearchTermEmptyValidationStatus(), keywordGroupSearchTerm); validationVisualizer.initVisualization(viewModel.keywordFieldEmptyValidationStatus(), keywordGroupSearchField); - validationVisualizer.initVisualization(viewModel.texGroupFilePathValidatonStatus(), texGroupFilePath); + validationVisualizer.initVisualization(viewModel.texGroupFilePathValidationStatus(), texGroupFilePath); nameField.requestFocus(); }); @@ -279,10 +279,14 @@ public void initialize() { viewModel.colorFieldProperty().setValue(IconTheme.getDefaultGroupColor()); return; } - List colorsOfSiblings = parentNode.getChildren().stream().map(child -> child.getGroup().getColor()) - .flatMap(Optional::stream) - .toList(); - Optional parentColor = parentGroup().getColor(); + + List colorsOfSiblings = parentNode.getChildren() + .stream() + .map(GroupTreeNode::getGroup) + .map(AbstractGroup::getColor) + .flatMap(Optional::stream) + .toList(); + Optional parentColor = parentNode.getGroup().getColor(); Color color; color = parentColor.map(value -> GroupColorPicker.generateColor(colorsOfSiblings, value)).orElseGet(() -> GroupColorPicker.generateColor(colorsOfSiblings)); viewModel.colorFieldProperty().setValue(color); diff --git a/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java b/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java index 297736b5ea5..a19c42c8a81 100644 --- a/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java @@ -266,7 +266,7 @@ private void setupValidation() { return false; } return FileUtil.getFileExtension(input) - .map(extension -> "aux".equalsIgnoreCase(extension)) + .map("aux"::equalsIgnoreCase) .orElse(false); } }, @@ -572,7 +572,7 @@ public ValidationStatus keywordSearchTermEmptyValidationStatus() { return keywordSearchTermEmptyValidator.getValidationStatus(); } - public ValidationStatus texGroupFilePathValidatonStatus() { + public ValidationStatus texGroupFilePathValidationStatus() { return texGroupFilePathValidator.getValidationStatus(); } diff --git a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java index 875d6ad4075..bcc4ab2deba 100644 --- a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java @@ -21,11 +21,7 @@ import org.jabref.gui.StateManager; import org.jabref.gui.icon.IconTheme; import org.jabref.gui.icon.JabRefIcon; -import org.jabref.gui.util.BackgroundTask; -import org.jabref.gui.util.CustomLocalDragboard; -import org.jabref.gui.util.DroppingMouseLocation; -import org.jabref.gui.util.TaskExecutor; -import org.jabref.gui.util.UiTaskExecutor; +import org.jabref.gui.util.*; import org.jabref.logic.groups.DefaultGroupsFactory; import org.jabref.logic.layout.format.LatexToUnicodeFormatter; import org.jabref.model.FieldChange; @@ -52,7 +48,7 @@ public class GroupNodeViewModel { - private final String displayName; + private String displayName; private final boolean isRoot; private final ObservableList children; private final BibDatabaseContext databaseContext; @@ -65,10 +61,11 @@ public class GroupNodeViewModel { private final BooleanBinding allSelectedEntriesMatched; private final TaskExecutor taskExecutor; private final CustomLocalDragboard localDragBoard; - private final ObservableList entriesList; private final PreferencesService preferencesService; private final InvalidationListener onInvalidatedGroup = listener -> refreshGroup(); + private final ObservableList entriesList; + public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager stateManager, TaskExecutor taskExecutor, GroupTreeNode groupNode, CustomLocalDragboard localDragBoard, PreferencesService preferencesService) { this.databaseContext = Objects.requireNonNull(databaseContext); this.taskExecutor = Objects.requireNonNull(taskExecutor); @@ -76,8 +73,8 @@ public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager state this.groupNode = Objects.requireNonNull(groupNode); this.localDragBoard = Objects.requireNonNull(localDragBoard); this.preferencesService = preferencesService; - - displayName = new LatexToUnicodeFormatter().format(groupNode.getName()); + LatexToUnicodeFormatter formatter = new LatexToUnicodeFormatter(); + groupNode.nameSubscription(name -> displayName = formatter.format(name)); isRoot = groupNode.isRoot(); if (groupNode.getGroup() instanceof AutomaticGroup automaticGroup) { children = automaticGroup.createSubgroups(this.databaseContext.getDatabase().getEntries()) @@ -95,20 +92,28 @@ public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager state hasChildren.bind(Bindings.isNotEmpty(children)); EasyBind.subscribe(preferencesService.getGroupsPreferences().displayGroupCountProperty(), shouldDisplay -> updateMatchedEntries()); + if (!stateManager.activeGroupProperty().isEmpty()) { + this.updateMatchedEntries(); + } stateManager.activeGroupProperty().subscribe(this::updateMatchedEntries); expandedProperty.set(groupNode.getGroup().isExpanded()); expandedProperty.addListener((observable, oldValue, newValue) -> groupNode.getGroup().setExpanded(newValue)); // Register listener - // The wrapper created by the FXCollections will set a weak listener on the wrapped list. This weak listener gets garbage collected. Hence, we need to maintain a reference to this list. + // The wrapper created by the FXCollections will set a weak listener on the wrapped list. + // This weak listener gets garbage collected. Hence, we need to maintain a reference to this list. entriesList = databaseContext.getDatabase().getEntries(); - entriesList.addListener(this::onDatabaseChanged); + entriesList.addListener(this::onEntriesChanged); - EasyObservableList selectedEntriesMatchStatus = EasyBind.map(stateManager.getSelectedEntries(), groupNode::matches); + this.groupNode.subscribeToDescendantChanged(node -> this.refreshGroup()); + EasyObservableList selectedEntriesMatchStatus = EasyBind.map( + stateManager.getSelectedEntries(), + groupNode::matches + ); anySelectedEntriesMatched = selectedEntriesMatchStatus.anyMatch(matched -> matched); // 'all' returns 'true' for empty streams, so this has to be checked explicitly - allSelectedEntriesMatched = selectedEntriesMatchStatus.isEmptyBinding().not().and(selectedEntriesMatchStatus.allMatch(matched -> matched)); + allSelectedEntriesMatched = selectedEntriesMatchStatus.allMatch(matched -> matched).and(selectedEntriesMatchStatus.isEmptyBinding().not()); } public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager stateManager, TaskExecutor taskExecutor, AbstractGroup group, CustomLocalDragboard localDragboard, PreferencesService preferencesService) { @@ -232,10 +237,10 @@ public GroupTreeNode getGroupNode() { /** * Gets invoked if an entry in the current database changes. */ - private void onDatabaseChanged(ListChangeListener.Change change) { + private void onEntriesChanged(ListChangeListener.Change change) { while (change.next()) { if (change.wasPermutated()) { - // Nothing to do, as permutation doesn't change matched entries + // do nothing } else if (change.wasUpdated()) { for (BibEntry changedEntry : change.getList().subList(change.getFrom(), change.getTo())) { if (groupNode.matches(changedEntry)) { @@ -251,10 +256,10 @@ private void onDatabaseChanged(ListChangeListener.Change cha matchedEntries.remove(removedEntry); } for (BibEntry addedEntry : change.getAddedSubList()) { + // Newly-created empty entries are indistinguishable from one another, + // so we should not check if the entry is already in our matched entries list. if (groupNode.matches(addedEntry)) { - if (!matchedEntries.contains(addedEntry)) { - matchedEntries.add(addedEntry); - } + matchedEntries.add(addedEntry); } } } @@ -495,7 +500,7 @@ public boolean isEditable() { } else if (group instanceof KeywordGroup) { // KeywordGroup is parent of LastNameGroup, RegexKeywordGroup and WordKeywordGroup return groupNode.getParent() - .map(parent -> parent.getGroup()) + .map(GroupTreeNode::getGroup) .map(groupParent -> !(groupParent instanceof AutomaticKeywordGroup || groupParent instanceof AutomaticPersonsGroup)) .orElse(false); } else if (group instanceof SearchGroup) { diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeView.java b/src/main/java/org/jabref/gui/groups/GroupTreeView.java index dd352998e67..933346ce42f 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeView.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeView.java @@ -9,6 +9,7 @@ import javafx.beans.binding.Bindings; import javafx.beans.property.ObjectProperty; +import javafx.collections.ObservableList; import javafx.css.PseudoClass; import javafx.geometry.Orientation; import javafx.scene.Node; @@ -159,8 +160,6 @@ private void createNodes() { this.setBottom(groupBar); } - - private void initialize() { this.localDragboard = stateManager.getLocalDragboard(); viewModel = new GroupTreeViewModel(stateManager, dialogService, preferencesService, taskExecutor, localDragboard); @@ -171,14 +170,19 @@ private void initialize() { BindingsHelper.bindContentBidirectional( groupTree.getSelectionModel().getSelectedItems(), - viewModel.selectedGroupsProperty(), - newSelectedGroups -> { - groupTree.getSelectionModel().clearSelection(); - newSelectedGroups.forEach(this::selectNode); - }, + stateManager.activeGroupProperty(), + this::selectActiveNodes, model -> { + // activeGroupProperty is read-only, so we update it by proxy + // by updating stateManager.getSelectedGroups(database), + // which propagates back to activeGroupProperty. if (stateManager.getActiveDatabase().isPresent()) { - updateSelection(model.stream().map(node -> node.getValue().getGroupNode()).collect(Collectors.toList()), stateManager.getActiveDatabase().get()); + updateSelection( + model.stream().map(TreeItem::getValue) + .map(GroupNodeViewModel::getGroupNode) + .collect(Collectors.toList()), + stateManager.getActiveDatabase().get() + ); } }); @@ -190,19 +194,23 @@ private void initialize() { }); searchField.textProperty().addListener((observable, oldValue, newValue) -> searchTask.restart()); - groupTree.rootProperty().bind( - EasyBind.map(viewModel.rootGroupProperty(), - group -> { - if (group == null) { - return null; - } else { - return new RecursiveTreeItem<>( - group, - GroupNodeViewModel::getChildren, - GroupNodeViewModel::expandedProperty, - viewModel.filterPredicateProperty()); - } - })); + // Binding these values causes some issues with update order which + // lead to the active groups not being highlighted. + EasyBind.subscribe(stateManager.activeDatabaseProperty(), + database -> { + viewModel.setNewDatabase(database); + groupTree.rootProperty().setValue( + viewModel.rootGroup() != null + ? new RecursiveTreeItem<>( + viewModel.rootGroup(), + GroupNodeViewModel::getChildren, + GroupNodeViewModel::expandedProperty, + viewModel.filterPredicateProperty()) + : null + ); + database.ifPresent(bibDatabaseContext -> selectActiveNodes(stateManager.activeGroupProperty())); + } + ); // Icon and group name new ViewModelTreeTableCellFactory() @@ -354,7 +362,7 @@ private void handleOnDragDropped(TreeTableRow row, GroupNode List changedGroups = new LinkedList<>(); for (String pathToSource : pathToSources) { Optional source = viewModel - .rootGroupProperty().get() + .rootGroup() .getChildByPath(pathToSource); if (source.isPresent() && source.get().canBeDragged()) { source.get().draggedOn(row.getItem(), ControlHelper.getDroppingMouseLocation(row, event)); @@ -394,12 +402,23 @@ private void handleOnDragOver(TreeTableRow row, GroupNodeVie } } - private void updateSelection(List newSelectedGroups, BibDatabaseContext database) { - if ((newSelectedGroups == null) || newSelectedGroups.isEmpty()) { - stateManager.getSelectedGroups(database).clear(); + private void updateSelection(List newActiveGroups, BibDatabaseContext database) { + if ((newActiveGroups == null)) { + stateManager.setSelectedGroups(database, Collections.singletonList(viewModel.rootGroup().getGroupNode())); } else { - List list = newSelectedGroups.stream().filter(model -> (model != null) && !(model.getGroup() instanceof AllEntriesGroup)).collect(Collectors.toList()); - stateManager.getSelectedGroups(database).setAll(list); + List list = newActiveGroups.stream() + .filter(model -> (model != null) && !(model.getGroup() instanceof AllEntriesGroup)) + .collect(Collectors.toList()); + stateManager.setSelectedGroups(database, list); + } + } + + private void selectActiveNodes(ObservableList activeGroups) { + groupTree.getSelectionModel().clearSelection(); + if (activeGroups != null && !activeGroups.isEmpty()) { + activeGroups.forEach(this::selectNode); + } else if (viewModel.rootGroup() != null) { + this.selectNode(viewModel.rootGroup().getGroupNode(), true); } } diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java index 2c9c257255b..b133fb7c8c9 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java @@ -1,23 +1,15 @@ package org.jabref.gui.groups; import java.util.ArrayList; -import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.function.Predicate; -import com.tobiasdiez.easybind.Subscription; -import javafx.beans.property.ListProperty; -import javafx.beans.property.ObjectProperty; -import javafx.beans.property.SimpleListProperty; -import javafx.beans.property.SimpleObjectProperty; -import javafx.beans.property.SimpleStringProperty; -import javafx.beans.property.StringProperty; +import javafx.beans.property.*; import javafx.collections.FXCollections; import javafx.collections.ListChangeListener; -import javafx.collections.ObservableList; import org.jabref.gui.AbstractViewModel; import org.jabref.gui.DialogService; @@ -69,7 +61,6 @@ public class GroupTreeViewModel extends AbstractViewModel { return 0; }; private Optional currentDatabase = Optional.empty(); - private Optional unsubscribe = Optional.empty(); public GroupTreeViewModel(StateManager stateManager, @@ -83,39 +74,29 @@ public GroupTreeViewModel(StateManager stateManager, this.taskExecutor = Objects.requireNonNull(taskExecutor); this.localDragboard = Objects.requireNonNull(localDragboard); - // Register listener - EasyBind.subscribe( - stateManager.activeDatabaseProperty(), - val -> this.onActiveDatabaseChanged() - ); - // Set-up bindings filterPredicate.bind(EasyBind.map(filterText, text -> group -> group.isMatchedBy(text))); - // Init - onActiveDatabaseChanged(); - } + ReadOnlyListProperty list = stateManager.activeGroupProperty(); + selectedGroups.setAll(list); + list.addListener(this::onActiveGroupChange); - private void subscribeToSelectedGroups() { - unsubscribe.ifPresent(Subscription::unsubscribe); - if (currentDatabase.isEmpty()) return; - ObservableList list = stateManager.getSelectedGroups(currentDatabase.get()); - list.addListener(this::onSelectedGroupChanged); - unsubscribe = Optional.of(() -> list.removeListener(this::onSelectedGroupChanged)); + // Init + setNewDatabase(currentDatabase); } /** * Gets invoked if the user selects a different group. * We need to notify the {@link StateManager} about this change so that the main table gets updated. */ - private void onSelectedGroupChanged(ListChangeListener.Change newValue) { + private void onActiveGroupChange(ListChangeListener.Change newValue) { if (!currentDatabase.equals(stateManager.activeDatabaseProperty().getValue())) { // Active database is not this one: do nothing return; } currentDatabase.ifPresent(database -> { - if ((newValue == null) || (newValue.getList().isEmpty())) { + if ((newValue == null)) { selectedGroups.clear(); } else { selectedGroups.setAll(newValue.getList()); @@ -123,18 +104,16 @@ private void onSelectedGroupChanged(ListChangeListener.Change newDatabase = stateManager.activeDatabaseProperty().getValue(); - currentDatabase = newDatabase; - newDatabase.ifPresentOrElse( + public void setNewDatabase(Optional database) { + currentDatabase = database; + database.ifPresentOrElse( this::onActiveDatabaseExists, this::onActiveDatabaseNull ); - subscribeToSelectedGroups(); } - public ObjectProperty rootGroupProperty() { - return rootGroup; + public GroupNodeViewModel rootGroup() { + return rootGroup.get(); } public ListProperty selectedGroupsProperty() { @@ -166,7 +145,7 @@ public void addNewGroupToRoot(boolean preferSelectedEntries) { /** * Gets invoked if the user changes the active database. - * We need to get the new group tree and update the view + * We need to get the new group tree */ private void onActiveDatabaseExists(BibDatabaseContext newDatabase) { @@ -175,11 +154,6 @@ private void onActiveDatabaseExists(BibDatabaseContext newDatabase) { .orElse(GroupNodeViewModel.getAllEntriesGroup(newDatabase, stateManager, taskExecutor, localDragboard, preferences)); rootGroup.setValue(newRoot); - if (stateManager.getSelectedGroups(newDatabase).isEmpty()) { - stateManager.setSelectedGroups(newDatabase, Collections.singletonList(newRoot.getGroupNode())); - } - selectedGroups.setAll( - stateManager.getSelectedGroups(newDatabase)); } private void onActiveDatabaseNull() { diff --git a/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java b/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java index 980360e9abc..a161d00325c 100644 --- a/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java +++ b/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java @@ -96,18 +96,20 @@ else if (database.isPresent()) )); newGroup.ifPresent(group -> { + GroupTreeNode newSubgroup; if (editGroup != null) { - groupEditActions(editGroup, group, database.get()); - return; + Optional editedGroup = groupEditActions(editGroup, group, database.get()); + if (editedGroup.isEmpty()) return; + newSubgroup = editedGroup.get(); + dialogService.notify(Localization.lang("Modified group \"%0\".", newSubgroup.getName())); + } else { + newSubgroup = groupFallsUnder.addSubgroup(group); + dialogService.notify(Localization.lang("Added group \"%0\".", newSubgroup.getName())); } - GroupTreeNode newSubgroup = groupFallsUnder.addSubgroup(group); stateManager.getSelectedGroups(database.get()).setAll(newSubgroup); - // TODO: Add undo // UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(parent, new GroupTreeNodeViewModel(newGroupNode), UndoableAddOrRemoveGroup.ADD_NODE); // panel.getUndoManager().addEdit(undo); - - dialogService.notify(Localization.lang("Added group \"%0\".", group.getName())); writeGroupChangesToMetaData(database.get(), newSubgroup); }); } else { @@ -116,12 +118,12 @@ else if (database.isPresent()) } - private void groupEditActions(GroupTreeNode oldGroup, AbstractGroup newGroup, BibDatabaseContext database) { - AbstractGroup oldGroupDef = oldGroup.getGroup(); - String oldGroupName = oldGroupDef.getName(); + private Optional groupEditActions(GroupTreeNode node, AbstractGroup newGroup, BibDatabaseContext database) { + AbstractGroup oldGroup = node.getGroup(); + String oldGroupName = oldGroup.getName(); - boolean groupTypeEqual = isGroupTypeEqual(oldGroupDef, newGroup); - boolean onlyMinorModifications = groupTypeEqual && changesAreMinor(oldGroupDef, newGroup); + boolean groupTypeEqual = isGroupTypeEqual(oldGroup, newGroup); + boolean onlyMinorModifications = groupTypeEqual && changesAreMinor(oldGroup, newGroup); // I inherited this from the previous maintainer. Haven't dug into which has better priority, // what default behavior is, or why this appears replicated -- all my changes are simply refactoring. @@ -138,7 +140,7 @@ private void groupEditActions(GroupTreeNode oldGroup, AbstractGroup newGroup, Bi // Otherwise, we need to keep them. removePreviousAssignments = nameIsUnique(oldGroup, newGroup, database); - } else if (groupTypeEqual && changesAreMinor(oldGroup.getGroup(), newGroup)) { + } else if (groupTypeEqual && changesAreMinor(oldGroup, newGroup)) { keepPreviousAssignments = true; removePreviousAssignments = true; @@ -147,9 +149,9 @@ private void groupEditActions(GroupTreeNode oldGroup, AbstractGroup newGroup, Bi // Major modifications Optional reassignmentResponse = showConfirmationPanel(newGroup.getClass() == WordKeywordGroup.class); if (reassignmentResponse.isEmpty() || reassignmentResponse.get().getButtonData() == ButtonBar.ButtonData.CANCEL_CLOSE) { - return; + return Optional.empty(); } - removePreviousAssignments = (oldGroup.getGroup() instanceof ExplicitGroup) + removePreviousAssignments = (node.getGroup() instanceof ExplicitGroup) && (newGroup instanceof ExplicitGroup) && nameIsUnique(oldGroup, newGroup, database); keepPreviousAssignments = (reassignmentResponse.get().getButtonData() == ButtonBar.ButtonData.YES); @@ -173,15 +175,12 @@ private void groupEditActions(GroupTreeNode oldGroup, AbstractGroup newGroup, Bi // undoAddPreviousEntries = UndoableChangeEntriesOfGroup.getUndoableEdit(null, addChange); // } - oldGroup.setGroup( + node.setGroup( newGroup, keepPreviousAssignments, removePreviousAssignments, database.getEntries()); - stateManager.getSelectedGroups(database).setAll(oldGroup); - dialogService.notify(Localization.lang("Modified group \"%0\".", oldGroup.getName())); - writeGroupChangesToMetaData(database, oldGroup); - + return Optional.of(node); } private boolean isGroupTypeEqual(AbstractGroup oldGroup, AbstractGroup newGroup) { @@ -196,7 +195,7 @@ private boolean isGroupTypeEqual(AbstractGroup oldGroup, AbstractGroup newGroup) * @param newGroup Edited group * @return true if just trivial modifications (e.g. color or description) or the relevant group properties are equal, false otherwise */ - private boolean changesAreMinor(AbstractGroup oldGroup, AbstractGroup newGroup) { + public static boolean changesAreMinor(AbstractGroup oldGroup, AbstractGroup newGroup) { // we need to use getclass here because we have different subclass inheritance e.g. ExplicitGroup is a subclass of WordKeyWordGroup if (oldGroup.getClass() == WordKeywordGroup.class) { WordKeywordGroup oldWordKeywordGroup = (WordKeywordGroup) oldGroup; @@ -238,7 +237,7 @@ private boolean changesAreMinor(AbstractGroup oldGroup, AbstractGroup newGroup) return true; } - private boolean nameIsUnique(GroupTreeNode oldGroup, AbstractGroup newGroup, BibDatabaseContext database) { + private boolean nameIsUnique(AbstractGroup oldGroup, AbstractGroup newGroup, BibDatabaseContext database) { int groupsWithSameName = 0; Optional databaseRootGroup = database.getMetaData().getGroups(); if (databaseRootGroup.isPresent()) { diff --git a/src/main/java/org/jabref/logic/database/DatabaseMerger.java b/src/main/java/org/jabref/logic/database/DatabaseMerger.java index 013b2e141b6..1c0868439e9 100644 --- a/src/main/java/org/jabref/logic/database/DatabaseMerger.java +++ b/src/main/java/org/jabref/logic/database/DatabaseMerger.java @@ -105,11 +105,12 @@ public void mergeMetaData(MetaData target, MetaData other, String otherFilename, private void mergeGroups(MetaData target, MetaData other, String otherFilename, List allOtherEntries) { // Adds the specified node as a child of the current root. The group contained in newGroups must not be of // type AllEntriesGroup, since every tree has exactly one AllEntriesGroup (its root). The newGroups are - // inserted directly, i.e. they are not deepCopy()'d. - other.getGroups().ifPresent(newGroups -> { + // deepCopy()'d by proxy through the GroupTreeNode's 'setGroup' function. This should allow 'undos' + // to be possible, if implemented. + other.getGroups().ifPresent(node -> { // ensure that there is always only one AllEntriesGroup in the resulting database // "Rename" the AllEntriesGroup of the imported database to "Imported" - if (newGroups.getGroup() instanceof AllEntriesGroup) { + if (node.getGroup() instanceof AllEntriesGroup) { // create a dummy group try { // This will cause a bug if the group already exists @@ -119,16 +120,16 @@ private void mergeGroups(MetaData target, MetaData other, String otherFilename, "Imported " + newGroupName, GroupHierarchyType.INDEPENDENT, keywordDelimiter); - newGroups.setGroup(group); group.add(allOtherEntries); + node.setGroup(group, true, false, allOtherEntries); } catch (IllegalArgumentException e) { LOGGER.error("Problem appending entries to group", e); } } target.getGroups().ifPresentOrElse( - newGroups::moveTo, + node::moveTo, // target does not contain any groups, so we can just use the new groups - () -> target.setGroups(newGroups)); + () -> target.setGroups(node)); }); } diff --git a/src/main/java/org/jabref/model/database/BibDatabaseContext.java b/src/main/java/org/jabref/model/database/BibDatabaseContext.java index 6e523fa29e3..08c83de5f04 100644 --- a/src/main/java/org/jabref/model/database/BibDatabaseContext.java +++ b/src/main/java/org/jabref/model/database/BibDatabaseContext.java @@ -9,6 +9,7 @@ import java.util.UUID; import java.util.stream.Collectors; +import javafx.collections.ObservableList; import org.jabref.architecture.AllowedToUseLogic; import org.jabref.gui.LibraryTab; import org.jabref.gui.desktop.JabRefDesktop; @@ -242,7 +243,7 @@ public void convertToLocalDatabase() { this.location = DatabaseLocation.LOCAL; } - public List getEntries() { + public ObservableList getEntries() { return database.getEntries(); } diff --git a/src/main/java/org/jabref/model/groups/AbstractGroup.java b/src/main/java/org/jabref/model/groups/AbstractGroup.java index 5f0aa7cbcfe..669d069c7f0 100644 --- a/src/main/java/org/jabref/model/groups/AbstractGroup.java +++ b/src/main/java/org/jabref/model/groups/AbstractGroup.java @@ -3,11 +3,15 @@ import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.function.Consumer; +import javafx.beans.property.IntegerProperty; +import javafx.beans.property.SimpleIntegerProperty; import javafx.beans.property.SimpleStringProperty; import javafx.beans.property.StringProperty; import javafx.scene.paint.Color; +import javafx.util.Subscription; import org.jabref.model.entry.BibEntry; import org.jabref.model.search.SearchMatcher; import org.jabref.model.strings.StringUtil; @@ -29,6 +33,7 @@ public abstract class AbstractGroup implements SearchMatcher { protected boolean isExpanded = true; protected Optional description = Optional.empty(); protected Optional iconName = Optional.empty(); + protected IntegerProperty versionNumber = new SimpleIntegerProperty(); protected AbstractGroup(String name, GroupHierarchyType context) { this.name.setValue(name); @@ -75,7 +80,7 @@ public void setColor(Color color) { public void setColor(String colorString) { if (StringUtil.isBlank(colorString)) { - color = Optional.empty(); + setColor((Color) null); } else { setColor(Color.valueOf(colorString)); } @@ -136,6 +141,14 @@ public StringProperty nameProperty() { */ public abstract boolean contains(BibEntry entry); + public Subscription subscribe(Consumer action) { + return versionNumber.subscribe(() -> action.accept(this.deepCopy())); + } + + protected void alertChange() { + versionNumber.set(versionNumber.get() + 1); + } + @Override public boolean isMatch(BibEntry entry) { return contains(entry); diff --git a/src/main/java/org/jabref/model/groups/GroupTreeNode.java b/src/main/java/org/jabref/model/groups/GroupTreeNode.java index a80ef1f159a..7d998385833 100644 --- a/src/main/java/org/jabref/model/groups/GroupTreeNode.java +++ b/src/main/java/org/jabref/model/groups/GroupTreeNode.java @@ -6,8 +6,14 @@ import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.function.Consumer; import java.util.stream.Collectors; +import javafx.beans.property.ObjectProperty; +import javafx.beans.property.SimpleObjectProperty; +import javafx.beans.property.SimpleStringProperty; +import javafx.beans.property.StringProperty; +import javafx.util.Subscription; import org.jabref.model.FieldChange; import org.jabref.model.TreeNode; import org.jabref.model.database.BibDatabase; @@ -22,7 +28,8 @@ public class GroupTreeNode extends TreeNode { private static final String PATH_DELIMITER = " > "; - private AbstractGroup group; + private final ObjectProperty group; + private final StringProperty groupName; /** * Creates this node and associates the specified group with it. @@ -31,7 +38,8 @@ public class GroupTreeNode extends TreeNode { */ public GroupTreeNode(AbstractGroup group) { super(GroupTreeNode.class); - setGroup(group, false, false, null); + this.group = new SimpleObjectProperty<>(Objects.requireNonNull(group)); + this.groupName = new SimpleStringProperty(group.getName()); } public static GroupTreeNode fromGroup(AbstractGroup group) { @@ -44,18 +52,7 @@ public static GroupTreeNode fromGroup(AbstractGroup group) { * @return the group associated with this node */ public AbstractGroup getGroup() { - return group; - } - - /** - * Associates the specified group with this node. - * - * @param newGroup the new group (has to be non-null) - * @deprecated use {@link #setGroup(AbstractGroup, boolean, boolean, List)}} instead - */ - @Deprecated - public void setGroup(AbstractGroup newGroup) { - this.group = Objects.requireNonNull(newGroup); + return group.get(); } /** @@ -68,10 +65,10 @@ public void setGroup(AbstractGroup newGroup) { */ public List setGroup(AbstractGroup newGroup, boolean shouldKeepPreviousAssignments, boolean shouldRemovePreviousAssignments, List entriesInDatabase) { + Objects.requireNonNull(newGroup); AbstractGroup oldGroup = getGroup(); - group = Objects.requireNonNull(newGroup); - List changes = new ArrayList<>(); + boolean shouldRemoveFromOldGroup = shouldRemovePreviousAssignments && (oldGroup instanceof GroupEntryChanger); boolean shouldAddToNewGroup = shouldKeepPreviousAssignments && (newGroup instanceof GroupEntryChanger); if (shouldAddToNewGroup || shouldRemoveFromOldGroup) { @@ -87,6 +84,9 @@ public List setGroup(AbstractGroup newGroup, boolean shouldKeepPrev changes.addAll(entryChanger.add(entriesMatchedByOldGroup)); } } + + group.set(newGroup); + groupName.set(newGroup.getName()); return changes; } @@ -94,17 +94,17 @@ public List setGroup(AbstractGroup newGroup, boolean shouldKeepPrev * Creates a {@link SearchMatcher} that matches entries of this group and that takes the hierarchical information into account. I.e., it finds elements contained in this nodes group, or the union of those elements in its own group and its children's groups (recursively), or the intersection of the elements in its own group and its parent's group (depending on the hierarchical settings stored in the involved groups) */ public SearchMatcher getSearchMatcher() { - return getSearchMatcher(group.getHierarchicalContext()); + return getSearchMatcher(group.get().getHierarchicalContext()); } private SearchMatcher getSearchMatcher(GroupHierarchyType originalContext) { - final GroupHierarchyType context = group.getHierarchicalContext(); + final GroupHierarchyType context = group.get().getHierarchicalContext(); if (context == GroupHierarchyType.INDEPENDENT) { - return group; + return group.get(); } MatcherSet searchRule = MatcherSets.build( context == GroupHierarchyType.REFINING ? MatcherSets.MatcherType.AND : MatcherSets.MatcherType.OR); - searchRule.addRule(group); + searchRule.addRule(group.get()); if ((context == GroupHierarchyType.INCLUDING) && (originalContext != GroupHierarchyType.REFINING)) { for (GroupTreeNode child : getChildren()) { searchRule.addRule(child.getSearchMatcher(originalContext)); @@ -126,13 +126,13 @@ public boolean equals(Object o) { return false; } GroupTreeNode that = (GroupTreeNode) o; - return Objects.equals(group, that.group) && + return Objects.equals(group.get(), that.group.get()) && Objects.equals(getChildren(), that.getChildren()); } @Override public int hashCode() { - return Objects.hash(group); + return Objects.hash(group.get()); } /** @@ -147,11 +147,11 @@ public List getContainingGroups(List entries, boolean r // Add myself if I contain the entries if (requireAll) { - if (this.group.containsAll(entries)) { + if (this.group.get().containsAll(entries)) { groups.add(this); } } else { - if (this.group.containsAny(entries)) { + if (this.group.get().containsAny(entries)) { groups.add(this); } } @@ -197,7 +197,7 @@ public List getMatchingGroups(List entries) { public List getEntriesInGroup(List entries) { List result = new ArrayList<>(); for (BibEntry entry : entries) { - if (this.group.contains(entry)) { + if (this.group.get().contains(entry)) { result.add(entry); } } @@ -210,7 +210,11 @@ public List getEntriesInGroup(List entries) { * @return String the name of the group */ public String getName() { - return group.getName(); + return group.get().getName(); + } + + public Subscription nameSubscription(Consumer consumer) { + return this.groupName.subscribe(consumer); } public GroupTreeNode addSubgroup(AbstractGroup subgroup) { @@ -221,7 +225,7 @@ public GroupTreeNode addSubgroup(AbstractGroup subgroup) { @Override public GroupTreeNode copyNode() { - return GroupTreeNode.fromGroup(group); + return GroupTreeNode.fromGroup(group.get()); } /** diff --git a/src/main/java/org/jabref/model/groups/RegexKeywordGroup.java b/src/main/java/org/jabref/model/groups/RegexKeywordGroup.java index dc603ffa8e5..29f372fe0f2 100644 --- a/src/main/java/org/jabref/model/groups/RegexKeywordGroup.java +++ b/src/main/java/org/jabref/model/groups/RegexKeywordGroup.java @@ -11,7 +11,7 @@ * Matches entries if the content of a given field is matched by a regular expression. */ public class RegexKeywordGroup extends KeywordGroup { - private Pattern pattern; + private final Pattern pattern; public RegexKeywordGroup(String name, GroupHierarchyType context, Field searchField, String searchExpression, boolean caseSensitive) { diff --git a/src/test/java/org/jabref/gui/groups/GroupDialogViewModelTest.java b/src/test/java/org/jabref/gui/groups/GroupDialogViewModelTest.java index 1c47527d0f8..8bd6f97d840 100644 --- a/src/test/java/org/jabref/gui/groups/GroupDialogViewModelTest.java +++ b/src/test/java/org/jabref/gui/groups/GroupDialogViewModelTest.java @@ -62,14 +62,14 @@ void validateExistingAbsolutePath() throws Exception { when(metaData.getLatexFileDirectory(any(String.class))).thenReturn(Optional.of(temporaryFolder)); viewModel.texGroupFilePathProperty().setValue(anAuxFile.toString()); - assertTrue(viewModel.texGroupFilePathValidatonStatus().isValid()); + assertTrue(viewModel.texGroupFilePathValidationStatus().isValid()); } @Test void validateNonExistingAbsolutePath() { var notAnAuxFile = temporaryFolder.resolve("notanauxfile.aux").toAbsolutePath(); viewModel.texGroupFilePathProperty().setValue(notAnAuxFile.toString()); - assertFalse(viewModel.texGroupFilePathValidatonStatus().isValid()); + assertFalse(viewModel.texGroupFilePathValidationStatus().isValid()); } @Test @@ -81,7 +81,7 @@ void validateExistingRelativePath() throws Exception { when(metaData.getLatexFileDirectory(any(String.class))).thenReturn(Optional.of(temporaryFolder)); viewModel.texGroupFilePathProperty().setValue(anAuxFile.toString()); - assertTrue(viewModel.texGroupFilePathValidatonStatus().isValid()); + assertTrue(viewModel.texGroupFilePathValidationStatus().isValid()); } @Test diff --git a/src/test/java/org/jabref/gui/groups/GroupTreeViewModelTest.java b/src/test/java/org/jabref/gui/groups/GroupTreeViewModelTest.java index 621b7465ec5..b89cf59e5e9 100644 --- a/src/test/java/org/jabref/gui/groups/GroupTreeViewModelTest.java +++ b/src/test/java/org/jabref/gui/groups/GroupTreeViewModelTest.java @@ -4,6 +4,7 @@ import org.jabref.gui.DialogService; import org.jabref.gui.StateManager; +import org.jabref.gui.maintable.CreateGroupAction; import org.jabref.gui.util.CurrentThreadTaskExecutor; import org.jabref.gui.util.CustomLocalDragboard; import org.jabref.gui.util.TaskExecutor; @@ -56,12 +57,12 @@ void setUp() { @Test void rootGroupIsAllEntriesByDefault() { AllEntriesGroup allEntriesGroup = new AllEntriesGroup("All entries"); - assertEquals(new GroupNodeViewModel(databaseContext, stateManager, taskExecutor, allEntriesGroup, new CustomLocalDragboard(), preferencesService), groupTree.rootGroupProperty().getValue()); + assertEquals(new GroupNodeViewModel(databaseContext, stateManager, taskExecutor, allEntriesGroup, new CustomLocalDragboard(), preferencesService), groupTree.rootGroup()); } @Test void rootGroupIsSelectedByDefault() { - assertEquals(groupTree.rootGroupProperty().get().getGroupNode(), stateManager.getSelectedGroups(databaseContext).getFirst()); + assertEquals(groupTree.rootGroup().getGroupNode(), stateManager.getSelectedGroups(databaseContext).getFirst()); } @Test @@ -97,9 +98,7 @@ void shouldNotShowDialogWhenGroupNameChanges() { AbstractGroup newGroup = new ExplicitGroup("newGroupName", GroupHierarchyType.INDEPENDENT, ','); BibEntry entry = new BibEntry(); databaseContext.getDatabase().insertEntry(entry); - - GroupTreeViewModel model = new GroupTreeViewModel(stateManager, dialogService, preferencesService, taskExecutor, new CustomLocalDragboard()); - assertTrue(model.onlyMinorChanges(oldGroup, newGroup)); + assertTrue(CreateGroupAction.changesAreMinor(oldGroup, newGroup)); } @Test @@ -109,9 +108,7 @@ void shouldNotShowDialogWhenGroupsAreEqual() { BibEntry entry = new BibEntry(); databaseContext.getDatabase().insertEntry(entry); - - GroupTreeViewModel model = new GroupTreeViewModel(stateManager, dialogService, preferencesService, taskExecutor, new CustomLocalDragboard()); - assertTrue(model.onlyMinorChanges(oldGroup, newGroup)); + assertTrue(CreateGroupAction.changesAreMinor(oldGroup, newGroup)); } @Test @@ -122,8 +119,7 @@ void shouldShowDialogWhenKeywordDiffers() { BibEntry entry = new BibEntry(); databaseContext.getDatabase().insertEntry(entry); - GroupTreeViewModel model = new GroupTreeViewModel(stateManager, dialogService, preferencesService, taskExecutor, new CustomLocalDragboard()); - assertFalse(model.onlyMinorChanges(oldGroup, newGroup)); + assertFalse(CreateGroupAction.changesAreMinor(oldGroup, newGroup)); } @Test @@ -133,8 +129,6 @@ void shouldShowDialogWhenCaseSensitivyDiffers() { BibEntry entry = new BibEntry(); databaseContext.getDatabase().insertEntry(entry); - - GroupTreeViewModel model = new GroupTreeViewModel(stateManager, dialogService, preferencesService, taskExecutor, new CustomLocalDragboard()); - assertFalse(model.onlyMinorChanges(oldGroup, newGroup)); + assertFalse(CreateGroupAction.changesAreMinor(oldGroup, newGroup)); } } From 8dd474a0ab498ee5b293638a0023357e8d9cc932 Mon Sep 17 00:00:00 2001 From: m-peeler Date: Tue, 9 Jul 2024 18:30:57 -0400 Subject: [PATCH 08/24] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0585e30f0d6..e67001727c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - We added support for using BibTeX Style files (BST) in the Preview. [#11102](https://github.com/JabRef/jabref/issues/11102) - We added support for automatically update LaTeX citations when a LaTeX file is created, removed, or modified. [#10585](https://github.com/JabRef/jabref/issues/10585) - We added the ability to collect a group by your current selection. [#11449](https://github.com/JabRef/jabref/issues/11449) +- We added the ability to right-click on selected entries and create a group using them. ### Changed From 53f1544d9620b2a73f9ca5724b427a5dfb6e0feb Mon Sep 17 00:00:00 2001 From: m-peeler Date: Tue, 9 Jul 2024 18:38:39 -0400 Subject: [PATCH 09/24] Update CHANGELOG.md --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1106b4542dc..de03f86d2c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,10 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv ### Added +- We added the ability to add the current selection to a newly created group. [#11449](https://github.com/JabRef/jabref/issues/11449) - We made new groups automatically to focus upon creation. [#11449](https://github.com/JabRef/jabref/issues/11449) +- We added the ability to right-click on selected entries and create a group using them. [#11449](https://github.com/JabRef/jabref/issues/11449) + ### Changed @@ -40,8 +43,6 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - We added an exporter and improved the importer for Endnote XML format. [#11137](https://github.com/JabRef/jabref/issues/11137) - We added support for using BibTeX Style files (BST) in the Preview. [#11102](https://github.com/JabRef/jabref/issues/11102) - We added support for automatically update LaTeX citations when a LaTeX file is created, removed, or modified. [#10585](https://github.com/JabRef/jabref/issues/10585) -- We added the ability to collect a group by your current selection. [#11449](https://github.com/JabRef/jabref/issues/11449) -- We added the ability to right-click on selected entries and create a group using them. ### Changed @@ -50,7 +51,6 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - JabRef no longer downloads HTML files of websites when a PDF was not found. [#10149](https://github.com/JabRef/jabref/issues/10149) - We added the HTTP message (in addition to the response code) if an error is encountered. [#11341](https://github.com/JabRef/jabref/pull/11341) - We made label wrap text to fit view size when reviewing external group changes. [#11220](https://github.com/JabRef/jabref/issues/11220) -- We made new Groups automatically focus upon creation. [#11449](https://github.com/JabRef/jabref/issues/11449) ### Fixed From ec9b0812fc74de9ee9fc9d9130bcd8fcb08930ca Mon Sep 17 00:00:00 2001 From: Michael Peeler <111776404+m-peeler@users.noreply.github.com> Date: Wed, 10 Jul 2024 02:34:49 -0400 Subject: [PATCH 10/24] Update CHANGELOG.md Co-authored-by: Oliver Kopp --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de03f86d2c3..f11a36f6e27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,6 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - We added the ability to add the current selection to a newly created group. [#11449](https://github.com/JabRef/jabref/issues/11449) - We made new groups automatically to focus upon creation. [#11449](https://github.com/JabRef/jabref/issues/11449) -- We added the ability to right-click on selected entries and create a group using them. [#11449](https://github.com/JabRef/jabref/issues/11449) ### Changed From aabb4a3a23fb5c907bd0b11740fb3966ee79634f Mon Sep 17 00:00:00 2001 From: m-peeler Date: Wed, 10 Jul 2024 02:35:15 -0400 Subject: [PATCH 11/24] Fixed formatting issues & addressed test failures --- .../java/org/jabref/gui/StateManager.java | 7 +++- .../jabref/gui/exporter/ExportCommand.java | 1 - .../jabref/gui/groups/GroupNodeViewModel.java | 34 +++++++++------- .../org/jabref/gui/groups/GroupTreeView.java | 39 +++++++++++-------- .../jabref/gui/groups/GroupTreeViewModel.java | 32 +++++++++------ .../gui/maintable/CreateGroupAction.java | 37 +++++++++--------- .../jabref/gui/maintable/RightClickMenu.java | 1 - .../model/database/BibDatabaseContext.java | 2 +- .../jabref/model/groups/AbstractGroup.java | 13 ++++--- .../jabref/model/groups/AutomaticGroup.java | 4 +- .../model/groups/AutomaticKeywordGroup.java | 15 ++++--- .../jabref/model/groups/GroupTreeNode.java | 6 ++- src/main/resources/l10n/JabRef_en.properties | 2 + .../gui/groups/GroupNodeViewModelTest.java | 7 +++- .../gui/groups/GroupTreeViewModelTest.java | 4 +- 15 files changed, 120 insertions(+), 84 deletions(-) diff --git a/src/main/java/org/jabref/gui/StateManager.java b/src/main/java/org/jabref/gui/StateManager.java index 71fb9f0f96c..0c297da2672 100644 --- a/src/main/java/org/jabref/gui/StateManager.java +++ b/src/main/java/org/jabref/gui/StateManager.java @@ -75,8 +75,13 @@ public class StateManager { private final ObservableList searchHistory = FXCollections.observableArrayList(); public StateManager() { + BibDatabaseContext nullContext = new BibDatabaseContext(); + selectedGroups.put(nullContext.getUid(), FXCollections.observableArrayList()); activeGroups.bind(Bindings.valueAt( - selectedGroups, activeDatabase.orElseOpt(null).map(BibDatabaseContext::getUid)).orElse(FXCollections.emptyObservableList())); + selectedGroups, + activeDatabase + .orElseOpt(nullContext) + .map(BibDatabaseContext::getUid))); } public ObservableList getVisibleSidePaneComponents() { diff --git a/src/main/java/org/jabref/gui/exporter/ExportCommand.java b/src/main/java/org/jabref/gui/exporter/ExportCommand.java index 64352057c49..814d639199a 100644 --- a/src/main/java/org/jabref/gui/exporter/ExportCommand.java +++ b/src/main/java/org/jabref/gui/exporter/ExportCommand.java @@ -2,7 +2,6 @@ import java.io.IOException; import java.nio.file.Path; -import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.function.Supplier; diff --git a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java index bcc4ab2deba..1f1390bf4c0 100644 --- a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java @@ -21,7 +21,11 @@ import org.jabref.gui.StateManager; import org.jabref.gui.icon.IconTheme; import org.jabref.gui.icon.JabRefIcon; -import org.jabref.gui.util.*; +import org.jabref.gui.util.TaskExecutor; +import org.jabref.gui.util.CustomLocalDragboard; +import org.jabref.gui.util.BackgroundTask; +import org.jabref.gui.util.UiTaskExecutor; +import org.jabref.gui.util.DroppingMouseLocation; import org.jabref.logic.groups.DefaultGroupsFactory; import org.jabref.logic.layout.format.LatexToUnicodeFormatter; import org.jabref.model.FieldChange; @@ -77,11 +81,12 @@ public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager state groupNode.nameSubscription(name -> displayName = formatter.format(name)); isRoot = groupNode.isRoot(); if (groupNode.getGroup() instanceof AutomaticGroup automaticGroup) { - children = automaticGroup.createSubgroups(this.databaseContext.getDatabase().getEntries()) - .stream() - .map(this::toViewModel) - .sorted((group1, group2) -> group1.getDisplayName().compareToIgnoreCase(group2.getDisplayName())) - .collect(Collectors.toCollection(FXCollections::observableArrayList)); + children = automaticGroup + .createSubgroups(this.databaseContext.getDatabase().getEntries()) + .stream() + .map(this::toViewModel) + .sorted((group1, group2) -> group1.getDisplayName().compareToIgnoreCase(group2.getDisplayName())) + .collect(Collectors.toCollection(FXCollections::observableArrayList)); } else { children = EasyBind.mapBacked(groupNode.getChildren(), this::toViewModel); } @@ -92,7 +97,7 @@ public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager state hasChildren.bind(Bindings.isNotEmpty(children)); EasyBind.subscribe(preferencesService.getGroupsPreferences().displayGroupCountProperty(), shouldDisplay -> updateMatchedEntries()); - if (!stateManager.activeGroupProperty().isEmpty()) { + if (stateManager.activeGroupProperty() != null && !stateManager.activeGroupProperty().isEmpty()) { this.updateMatchedEntries(); } stateManager.activeGroupProperty().subscribe(this::updateMatchedEntries); @@ -106,7 +111,6 @@ public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager state entriesList = databaseContext.getDatabase().getEntries(); entriesList.addListener(this::onEntriesChanged); - this.groupNode.subscribeToDescendantChanged(node -> this.refreshGroup()); EasyObservableList selectedEntriesMatchStatus = EasyBind.map( stateManager.getSelectedEntries(), groupNode::matches @@ -198,7 +202,7 @@ public String toString() { return "GroupNodeViewModel{" + "displayName='" + displayName + '\'' + ", isRoot=" + isRoot + - ", icon='" + getIcon() + '\'' + + ", icon='" + getIcon().name() + '\'' + ", children=" + children + ", databaseContext=" + databaseContext + ", groupNode=" + groupNode + @@ -256,12 +260,14 @@ private void onEntriesChanged(ListChangeListener.Change chan matchedEntries.remove(removedEntry); } for (BibEntry addedEntry : change.getAddedSubList()) { - // Newly-created empty entries are indistinguishable from one another, - // so we should not check if the entry is already in our matched entries list. - if (groupNode.matches(addedEntry)) { - matchedEntries.add(addedEntry); + // Check for strict equality, as otherwise newly-created entries + // would all be considered the same. + if (matchedEntries.stream().anyMatch(e -> e == addedEntry)) { + if (groupNode.matches(addedEntry)) { + matchedEntries.add(addedEntry); + } + } } - } } } } diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeView.java b/src/main/java/org/jabref/gui/groups/GroupTreeView.java index 933346ce42f..6f124871d6b 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeView.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeView.java @@ -4,8 +4,13 @@ import java.lang.reflect.Method; import java.text.DecimalFormat; import java.time.Duration; -import java.util.*; +import java.util.Objects; import java.util.stream.Collectors; +import java.util.List; +import java.util.Optional; +import java.util.ArrayList; +import java.util.LinkedList; +import java.util.Collections; import javafx.beans.binding.Bindings; import javafx.beans.property.ObjectProperty; @@ -169,22 +174,22 @@ private void initialize() { dragExpansionHandler = new DragExpansionHandler(); BindingsHelper.bindContentBidirectional( - groupTree.getSelectionModel().getSelectedItems(), - stateManager.activeGroupProperty(), - this::selectActiveNodes, - model -> { - // activeGroupProperty is read-only, so we update it by proxy - // by updating stateManager.getSelectedGroups(database), - // which propagates back to activeGroupProperty. - if (stateManager.getActiveDatabase().isPresent()) { - updateSelection( - model.stream().map(TreeItem::getValue) - .map(GroupNodeViewModel::getGroupNode) - .collect(Collectors.toList()), - stateManager.getActiveDatabase().get() - ); - } - }); + groupTree.getSelectionModel().getSelectedItems(), + stateManager.activeGroupProperty(), + this::selectActiveNodes, + model -> { + // activeGroupProperty is read-only, so we update it by proxy + // by updating stateManager.getSelectedGroups(database), + // which propagates back to activeGroupProperty. + if (stateManager.getActiveDatabase().isPresent()) { + updateSelection( + model.stream().map(TreeItem::getValue) + .map(GroupNodeViewModel::getGroupNode) + .collect(Collectors.toList()), + stateManager.getActiveDatabase().get() + ); + } + }); // We try to prevent publishing changes in the search field directly to the search task that takes some time // for larger group structures. diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java index b133fb7c8c9..eb472af356d 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java @@ -1,13 +1,15 @@ package org.jabref.gui.groups; -import java.util.ArrayList; -import java.util.Comparator; -import java.util.List; -import java.util.Objects; -import java.util.Optional; +import java.util.*; import java.util.function.Predicate; -import javafx.beans.property.*; +import javafx.beans.property.ListProperty; +import javafx.beans.property.ObjectProperty; +import javafx.beans.property.SimpleListProperty; +import javafx.beans.property.SimpleObjectProperty; +import javafx.beans.property.StringProperty; +import javafx.beans.property.SimpleStringProperty; +import javafx.beans.property.ReadOnlyListProperty; import javafx.collections.FXCollections; import javafx.collections.ListChangeListener; @@ -62,7 +64,6 @@ public class GroupTreeViewModel extends AbstractViewModel { }; private Optional currentDatabase = Optional.empty(); - public GroupTreeViewModel(StateManager stateManager, DialogService dialogService, PreferencesService preferencesService, @@ -78,11 +79,18 @@ public GroupTreeViewModel(StateManager stateManager, filterPredicate.bind(EasyBind.map(filterText, text -> group -> group.isMatchedBy(text))); ReadOnlyListProperty list = stateManager.activeGroupProperty(); + Optional database = stateManager.getActiveDatabase(); selectedGroups.setAll(list); list.addListener(this::onActiveGroupChange); // Init - setNewDatabase(currentDatabase); + setNewDatabase(stateManager.getActiveDatabase()); + database.ifPresent( + bibDatabaseContext -> + stateManager.setSelectedGroups( + bibDatabaseContext, + Collections.singletonList(rootGroup.get().getGroupNode() + ))); } /** @@ -104,6 +112,10 @@ private void onActiveGroupChange(ListChangeListener.Change database) { currentDatabase = database; database.ifPresentOrElse( @@ -142,13 +154,11 @@ public void addNewGroupToRoot(boolean preferSelectedEntries) { groupMaker.execute(); } - /** * Gets invoked if the user changes the active database. * We need to get the new group tree */ private void onActiveDatabaseExists(BibDatabaseContext newDatabase) { - GroupNodeViewModel newRoot = newDatabase.getMetaData().getGroups() .map(root -> new GroupNodeViewModel(newDatabase, stateManager, taskExecutor, root, localDragboard, preferences)) .orElse(GroupNodeViewModel.getAllEntriesGroup(newDatabase, stateManager, taskExecutor, localDragboard, preferences)); @@ -316,7 +326,7 @@ void removeGroupsAndSubGroupsFromEntries(GroupTreeNode group) { } // only remove explicit groups from the entries, keyword groups should not be deleted - if (group.getGroup() instanceof ExplicitGroup) { + if (currentDatabase.isPresent() && group.getGroup() instanceof ExplicitGroup) { int groupsWithSameName = 0; String name = group.getGroup().getName(); Optional rootGroup = currentDatabase.get().getMetaData().getGroups(); diff --git a/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java b/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java index a161d00325c..68c37470881 100644 --- a/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java +++ b/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java @@ -1,9 +1,14 @@ package org.jabref.gui.maintable; -import javafx.scene.Group; import javafx.scene.control.Alert; import javafx.scene.control.ButtonBar; import javafx.scene.control.ButtonType; + +import org.jspecify.annotations.Nullable; + +import java.util.Objects; +import java.util.Optional; + import org.jabref.gui.DialogService; import org.jabref.gui.StateManager; import org.jabref.gui.actions.SimpleCommand; @@ -11,13 +16,16 @@ import org.jabref.gui.groups.GroupDialogView; import org.jabref.logic.l10n.Localization; import org.jabref.model.database.BibDatabaseContext; -import org.jabref.model.groups.*; -import org.jspecify.annotations.Nullable; +import org.jabref.model.groups.AbstractGroup; +import org.jabref.model.groups.ExplicitGroup; +import org.jabref.model.groups.WordKeywordGroup; +import org.jabref.model.groups.GroupTreeNode; +import org.jabref.model.groups.RegexKeywordGroup; +import org.jabref.model.groups.TexGroup; +import org.jabref.model.groups.SearchGroup; +import org.jabref.model.groups.AutomaticKeywordGroup; +import org.jabref.model.groups.AutomaticPersonsGroup; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Objects; -import java.util.Optional; import static org.jabref.gui.actions.ActionHelper.needsEntriesSelected; @@ -61,12 +69,13 @@ public void execute() { Optional database = stateManager.getActiveDatabase(); Optional active; - if (!stateManager.activeGroupProperty().isEmpty()) + if (!stateManager.activeGroupProperty().isEmpty()) { active = Optional.of(stateManager.activeGroupProperty().getFirst()); - else if (database.isPresent()) + } else if (database.isPresent()) { active = database.get().getMetaData().getGroups(); - else + } else { active = Optional.empty(); + } if (database.isPresent() && active.isPresent()) { boolean isRoot = active.get().isRoot(); @@ -115,7 +124,6 @@ else if (database.isPresent()) } else { dialogService.showWarningDialogAndWait(Localization.lang("Cannot create group"), Localization.lang("Cannot create group. Please create a library first.")); } - } private Optional groupEditActions(GroupTreeNode node, AbstractGroup newGroup, BibDatabaseContext database) { @@ -134,17 +142,13 @@ private Optional groupEditActions(GroupTreeNode node, AbstractGro // dialog already warns us about this if the new group is named like another existing group // We need to check if only the name changed as this is relevant for the entry's group field if (groupTypeEqual && !newGroup.getName().equals(oldGroupName) && onlyMinorModifications) { - keepPreviousAssignments = true; // If the name is unique, we want to remove the old database with the same name. // Otherwise, we need to keep them. removePreviousAssignments = nameIsUnique(oldGroup, newGroup, database); - } else if (groupTypeEqual && changesAreMinor(oldGroup, newGroup)) { - keepPreviousAssignments = true; removePreviousAssignments = true; - } else { // Major modifications Optional reassignmentResponse = showConfirmationPanel(newGroup.getClass() == WordKeywordGroup.class); @@ -156,8 +160,6 @@ private Optional groupEditActions(GroupTreeNode node, AbstractGro && nameIsUnique(oldGroup, newGroup, database); keepPreviousAssignments = (reassignmentResponse.get().getButtonData() == ButtonBar.ButtonData.YES); } - - // TODO: Add undo // Store undo information. // AbstractUndoableEdit undoAddPreviousEntries = null; @@ -174,7 +176,6 @@ private Optional groupEditActions(GroupTreeNode node, AbstractGro // if (!addChange.isEmpty()) { // undoAddPreviousEntries = UndoableChangeEntriesOfGroup.getUndoableEdit(null, addChange); // } - node.setGroup( newGroup, keepPreviousAssignments, diff --git a/src/main/java/org/jabref/gui/maintable/RightClickMenu.java b/src/main/java/org/jabref/gui/maintable/RightClickMenu.java index f37bd9f2d8f..803e7265e56 100644 --- a/src/main/java/org/jabref/gui/maintable/RightClickMenu.java +++ b/src/main/java/org/jabref/gui/maintable/RightClickMenu.java @@ -83,7 +83,6 @@ public static ContextMenu create(BibEntryTableViewModel entry, : StandardActions.CREATE_SUBGROUP_FROM_SELECTION, new CreateGroupAction(dialogService, stateManager, true, PreferredGroupAdditionLocation.ADD_BELOW, null)), - new SeparatorMenuItem(), factory.createMenuItem(StandardActions.ATTACH_FILE, new AttachFileAction(libraryTab, dialogService, stateManager, preferencesService.getFilePreferences())), diff --git a/src/main/java/org/jabref/model/database/BibDatabaseContext.java b/src/main/java/org/jabref/model/database/BibDatabaseContext.java index 08c83de5f04..d59d12ce6d9 100644 --- a/src/main/java/org/jabref/model/database/BibDatabaseContext.java +++ b/src/main/java/org/jabref/model/database/BibDatabaseContext.java @@ -8,8 +8,8 @@ import java.util.Optional; import java.util.UUID; import java.util.stream.Collectors; - import javafx.collections.ObservableList; + import org.jabref.architecture.AllowedToUseLogic; import org.jabref.gui.LibraryTab; import org.jabref.gui.desktop.JabRefDesktop; diff --git a/src/main/java/org/jabref/model/groups/AbstractGroup.java b/src/main/java/org/jabref/model/groups/AbstractGroup.java index 669d069c7f0..6c81ab0a078 100644 --- a/src/main/java/org/jabref/model/groups/AbstractGroup.java +++ b/src/main/java/org/jabref/model/groups/AbstractGroup.java @@ -10,8 +10,8 @@ import javafx.beans.property.SimpleStringProperty; import javafx.beans.property.StringProperty; import javafx.scene.paint.Color; - import javafx.util.Subscription; + import org.jabref.model.entry.BibEntry; import org.jabref.model.search.SearchMatcher; import org.jabref.model.strings.StringUtil; @@ -45,10 +45,10 @@ public String toString() { return "AbstractGroup{" + "name='" + name.getValue() + '\'' + ", context=" + context + - ", color=" + color + + ", color=" + color.orElse(Color.BLACK) + ", isExpanded=" + isExpanded + - ", description=" + description + - ", iconName=" + iconName + + ", description=" + description.orElse("null") + + ", iconName=" + iconName.orElse("null") + '}'; } @@ -61,13 +61,14 @@ public boolean equals(Object other) { return false; } AbstractGroup that = (AbstractGroup) other; - return Objects.equals(this.name.getValue(), that.name.getValue()) && Objects.equals(this.description, that.description) + return Objects.equals(this.name.getValue(), that.name.getValue()) + && Objects.equals(this.description, that.description) && Objects.equals(this.context, that.context); } @Override public int hashCode() { - return Objects.hash(name.getValue(), description, context); + return Objects.hash(name.getValue(), getDescription().orElse("null"), getHierarchicalContext()); } public Optional getColor() { diff --git a/src/main/java/org/jabref/model/groups/AutomaticGroup.java b/src/main/java/org/jabref/model/groups/AutomaticGroup.java index 026ce74f76f..f666b3ccffe 100644 --- a/src/main/java/org/jabref/model/groups/AutomaticGroup.java +++ b/src/main/java/org/jabref/model/groups/AutomaticGroup.java @@ -27,7 +27,7 @@ public boolean isDynamic() { public ObservableList createSubgroups(ObservableList entries) { // TODO: Propagate changes to entry list (however: there is no flatMap and collect as TransformationList) return entries.stream() - .flatMap(entry -> createSubgroups(entry).stream()) - .collect(TreeCollector.mergeIntoTree(GroupTreeNode::isSameGroupAs)); + .flatMap(entry -> createSubgroups(entry).stream()) + .collect(TreeCollector.mergeIntoTree(GroupTreeNode::isSameGroupAs)); } } diff --git a/src/main/java/org/jabref/model/groups/AutomaticKeywordGroup.java b/src/main/java/org/jabref/model/groups/AutomaticKeywordGroup.java index 06dfcd983c1..d1c44ff748a 100644 --- a/src/main/java/org/jabref/model/groups/AutomaticKeywordGroup.java +++ b/src/main/java/org/jabref/model/groups/AutomaticKeywordGroup.java @@ -49,8 +49,10 @@ public boolean equals(Object o) { return false; } AutomaticKeywordGroup that = (AutomaticKeywordGroup) o; - return Objects.equals(keywordDelimiter, that.keywordDelimiter) && - Objects.equals(field, that.field); + return + super.equals(that) + && Objects.equals(keywordDelimiter, that.keywordDelimiter) + && Objects.equals(field, that.field); } @Override @@ -61,10 +63,11 @@ public int hashCode() { @Override public Set createSubgroups(BibEntry entry) { KeywordList keywordList = entry.getFieldAsKeywords(field, keywordDelimiter); - return keywordList.stream() - .filter(keyword -> StringUtil.isNotBlank(keyword.get())) - .map(this::createGroup) - .collect(Collectors.toSet()); + Set nodes = keywordList.stream() + .filter(keyword -> StringUtil.isNotBlank(keyword.get())) + .map(this::createGroup) + .collect(Collectors.toSet()); + return nodes; } private GroupTreeNode createGroup(Keyword keywordChain) { diff --git a/src/main/java/org/jabref/model/groups/GroupTreeNode.java b/src/main/java/org/jabref/model/groups/GroupTreeNode.java index 7d998385833..dc4f1942fb8 100644 --- a/src/main/java/org/jabref/model/groups/GroupTreeNode.java +++ b/src/main/java/org/jabref/model/groups/GroupTreeNode.java @@ -14,6 +14,7 @@ import javafx.beans.property.SimpleStringProperty; import javafx.beans.property.StringProperty; import javafx.util.Subscription; + import org.jabref.model.FieldChange; import org.jabref.model.TreeNode; import org.jabref.model.database.BibDatabase; @@ -132,7 +133,8 @@ public boolean equals(Object o) { @Override public int hashCode() { - return Objects.hash(group.get()); + return group.hashCode(); + } /** @@ -326,6 +328,6 @@ public List removeEntriesFromGroup(List entries) { * Returns true if the underlying groups of both {@link GroupTreeNode}s is the same. */ public boolean isSameGroupAs(GroupTreeNode other) { - return Objects.equals(group, other.group); + return Objects.equals(group.get(), other.group.get()); } } diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index 59288b5ba7d..1a4149ea99c 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -2650,3 +2650,5 @@ Warning\:\ Failed\ to\ check\ if\ the\ directory\ is\ empty.=Warning: Failed to Warning\:\ The\ selected\ directory\ is\ not\ a\ valid\ directory.=Warning: The selected directory is not a valid directory. Current\ selection=Current selection Group\ all\ entries\ currently\ selected=Group all entries currently selected +Create\ group\ from\ selection=Create group from selection +Create\ subgroup\ from\ selection=Create subgroup from selection diff --git a/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java b/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java index f05b398d285..18ff93df52e 100644 --- a/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java +++ b/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java @@ -1,6 +1,7 @@ package org.jabref.gui.groups; import java.util.Arrays; +import java.util.stream.Collectors; import javafx.collections.FXCollections; import javafx.collections.ObservableList; @@ -40,7 +41,9 @@ class GroupNodeViewModelTest { @BeforeEach void setUp() { stateManager = mock(StateManager.class); + StateManager templateStateManager = new StateManager(); when(stateManager.getSelectedEntries()).thenReturn(FXCollections.emptyObservableList()); + when(stateManager.activeGroupProperty()).thenReturn(templateStateManager.activeGroupProperty()); databaseContext = new BibDatabaseContext(); taskExecutor = new CurrentThreadTaskExecutor(); preferencesService = mock(PreferencesService.class); @@ -96,9 +99,9 @@ void treeOfAutomaticKeywordGroupIsCombined() { expectedA.addSubgroup(expectedGroupC); expectedA.addSubgroup(expectedGroupD); GroupNodeViewModel expectedE = getViewModelForGroup(expectedGroupE); - ObservableList expected = FXCollections.observableArrayList(expectedA, expectedE); + ObservableList expected = FXCollections.observableArrayList(expectedA.getGroupNode().getGroup(), expectedE.getGroupNode().getGroup()); - assertEquals(expected, groupViewModel.getChildren()); + assertEquals(expected, groupViewModel.getGroupNode().getChildren().stream().map(GroupTreeNode::getGroup).collect(Collectors.toCollection(FXCollections::observableArrayList))); } @Test diff --git a/src/test/java/org/jabref/gui/groups/GroupTreeViewModelTest.java b/src/test/java/org/jabref/gui/groups/GroupTreeViewModelTest.java index b89cf59e5e9..5108f153d03 100644 --- a/src/test/java/org/jabref/gui/groups/GroupTreeViewModelTest.java +++ b/src/test/java/org/jabref/gui/groups/GroupTreeViewModelTest.java @@ -41,7 +41,7 @@ class GroupTreeViewModelTest { void setUp() { databaseContext = new BibDatabaseContext(); stateManager = new StateManager(); - stateManager.activeDatabaseProperty().setValue(Optional.of(databaseContext)); + stateManager.setActiveDatabase(databaseContext); taskExecutor = new CurrentThreadTaskExecutor(); preferencesService = mock(PreferencesService.class); dialogService = mock(DialogService.class, Answers.RETURNS_DEEP_STUBS); @@ -57,7 +57,7 @@ void setUp() { @Test void rootGroupIsAllEntriesByDefault() { AllEntriesGroup allEntriesGroup = new AllEntriesGroup("All entries"); - assertEquals(new GroupNodeViewModel(databaseContext, stateManager, taskExecutor, allEntriesGroup, new CustomLocalDragboard(), preferencesService), groupTree.rootGroup()); + assertEquals(new GroupNodeViewModel(databaseContext, stateManager, taskExecutor, allEntriesGroup, new CustomLocalDragboard(), preferencesService).getGroupNode(), groupTree.rootGroup().getGroupNode()); } @Test From d772aaaa72e35299c2bdc2b345247d42f1d986aa Mon Sep 17 00:00:00 2001 From: m-peeler Date: Wed, 10 Jul 2024 02:39:31 -0400 Subject: [PATCH 12/24] Fixed unnecessary module --- src/main/java/module-info.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/module-info.java b/src/main/java/module-info.java index 3e10d8dc7e9..d9a30a4a5b0 100644 --- a/src/main/java/module-info.java +++ b/src/main/java/module-info.java @@ -46,7 +46,7 @@ requires org.tinylog.impl; provides org.tinylog.writers.Writer - with org.jabref.gui.logging.GuiWriter; + with org.jabref.gui.logging.GuiWriter; // Preferences and XML requires java.prefs; @@ -150,5 +150,4 @@ requires dd.plist; requires mslinks; requires org.apache.httpcomponents.core5.httpcore5; - requires commons.beanutils; } From d3e1f1e2cbbe457a2208ea141509528056a21f00 Mon Sep 17 00:00:00 2001 From: m-peeler Date: Wed, 10 Jul 2024 16:46:18 -0400 Subject: [PATCH 13/24] Style & Bug Fixes Addressed failed tests and style guidelines --- .../jabref/gui/groups/GroupNodeViewModel.java | 17 ++++++++------- .../org/jabref/gui/groups/GroupTreeView.java | 10 ++++----- .../jabref/gui/groups/GroupTreeViewModel.java | 11 +++++++--- .../gui/maintable/CreateGroupAction.java | 21 ++++++++++--------- src/main/java/org/jabref/model/TreeNode.java | 1 - .../model/database/BibDatabaseContext.java | 1 + .../model/groups/AutomaticKeywordGroup.java | 3 +-- .../jabref/model/groups/GroupTreeNode.java | 7 ++++--- .../gui/groups/GroupNodeViewModelTest.java | 4 ++-- .../groups/AutomaticKeywordGroupTest.java | 11 ++++++++-- 10 files changed, 50 insertions(+), 36 deletions(-) diff --git a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java index 1f1390bf4c0..76ea6fac006 100644 --- a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java @@ -21,11 +21,11 @@ import org.jabref.gui.StateManager; import org.jabref.gui.icon.IconTheme; import org.jabref.gui.icon.JabRefIcon; -import org.jabref.gui.util.TaskExecutor; -import org.jabref.gui.util.CustomLocalDragboard; import org.jabref.gui.util.BackgroundTask; -import org.jabref.gui.util.UiTaskExecutor; +import org.jabref.gui.util.CustomLocalDragboard; import org.jabref.gui.util.DroppingMouseLocation; +import org.jabref.gui.util.TaskExecutor; +import org.jabref.gui.util.UiTaskExecutor; import org.jabref.logic.groups.DefaultGroupsFactory; import org.jabref.logic.layout.format.LatexToUnicodeFormatter; import org.jabref.model.FieldChange; @@ -87,6 +87,7 @@ public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager state .map(this::toViewModel) .sorted((group1, group2) -> group1.getDisplayName().compareToIgnoreCase(group2.getDisplayName())) .collect(Collectors.toCollection(FXCollections::observableArrayList)); + children.forEach(c -> groupNode.addChild(c.getGroupNode())); } else { children = EasyBind.mapBacked(groupNode.getChildren(), this::toViewModel); } @@ -94,7 +95,7 @@ public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager state databaseContext.getMetaData().groupsBinding().addListener(new WeakInvalidationListener(onInvalidatedGroup)); } hasChildren = new SimpleBooleanProperty(); - hasChildren.bind(Bindings.isNotEmpty(children)); + hasChildren.bind(Bindings.isNotEmpty(groupNode.getChildren())); EasyBind.subscribe(preferencesService.getGroupsPreferences().displayGroupCountProperty(), shouldDisplay -> updateMatchedEntries()); if (stateManager.activeGroupProperty() != null && !stateManager.activeGroupProperty().isEmpty()) { @@ -260,14 +261,14 @@ private void onEntriesChanged(ListChangeListener.Change chan matchedEntries.remove(removedEntry); } for (BibEntry addedEntry : change.getAddedSubList()) { - // Check for strict equality, as otherwise newly-created entries - // would all be considered the same. - if (matchedEntries.stream().anyMatch(e -> e == addedEntry)) { + // Check for ID equality, otherwise new entries with no field info aren't added + // and entry count doesn't increment. + if (matchedEntries.stream().noneMatch(e -> e.getId().equals(addedEntry.getId()))) { if (groupNode.matches(addedEntry)) { matchedEntries.add(addedEntry); } } - } + } } } } diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeView.java b/src/main/java/org/jabref/gui/groups/GroupTreeView.java index 6f124871d6b..f6f3b6c4431 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeView.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeView.java @@ -4,13 +4,13 @@ import java.lang.reflect.Method; import java.text.DecimalFormat; import java.time.Duration; -import java.util.Objects; -import java.util.stream.Collectors; -import java.util.List; -import java.util.Optional; import java.util.ArrayList; -import java.util.LinkedList; import java.util.Collections; +import java.util.LinkedList; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.stream.Collectors; import javafx.beans.binding.Bindings; import javafx.beans.property.ObjectProperty; diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java index eb472af356d..d40177bea9e 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java @@ -1,15 +1,20 @@ package org.jabref.gui.groups; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Objects; +import java.util.Optional; import java.util.function.Predicate; import javafx.beans.property.ListProperty; import javafx.beans.property.ObjectProperty; +import javafx.beans.property.ReadOnlyListProperty; import javafx.beans.property.SimpleListProperty; import javafx.beans.property.SimpleObjectProperty; -import javafx.beans.property.StringProperty; import javafx.beans.property.SimpleStringProperty; -import javafx.beans.property.ReadOnlyListProperty; +import javafx.beans.property.StringProperty; import javafx.collections.FXCollections; import javafx.collections.ListChangeListener; diff --git a/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java b/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java index 68c37470881..4443b0fde84 100644 --- a/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java +++ b/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java @@ -1,14 +1,12 @@ package org.jabref.gui.maintable; +import java.util.Objects; +import java.util.Optional; + import javafx.scene.control.Alert; import javafx.scene.control.ButtonBar; import javafx.scene.control.ButtonType; -import org.jspecify.annotations.Nullable; - -import java.util.Objects; -import java.util.Optional; - import org.jabref.gui.DialogService; import org.jabref.gui.StateManager; import org.jabref.gui.actions.SimpleCommand; @@ -17,15 +15,16 @@ import org.jabref.logic.l10n.Localization; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.groups.AbstractGroup; +import org.jabref.model.groups.AutomaticKeywordGroup; +import org.jabref.model.groups.AutomaticPersonsGroup; import org.jabref.model.groups.ExplicitGroup; -import org.jabref.model.groups.WordKeywordGroup; import org.jabref.model.groups.GroupTreeNode; import org.jabref.model.groups.RegexKeywordGroup; -import org.jabref.model.groups.TexGroup; import org.jabref.model.groups.SearchGroup; -import org.jabref.model.groups.AutomaticKeywordGroup; -import org.jabref.model.groups.AutomaticPersonsGroup; +import org.jabref.model.groups.TexGroup; +import org.jabref.model.groups.WordKeywordGroup; +import org.jspecify.annotations.Nullable; import static org.jabref.gui.actions.ActionHelper.needsEntriesSelected; @@ -108,7 +107,9 @@ public void execute() { GroupTreeNode newSubgroup; if (editGroup != null) { Optional editedGroup = groupEditActions(editGroup, group, database.get()); - if (editedGroup.isEmpty()) return; + if (editedGroup.isEmpty()) { + return; + } newSubgroup = editedGroup.get(); dialogService.notify(Localization.lang("Modified group \"%0\".", newSubgroup.getName())); } else { diff --git a/src/main/java/org/jabref/model/TreeNode.java b/src/main/java/org/jabref/model/TreeNode.java index 0ba3ea1a204..fcf0b8ab26e 100644 --- a/src/main/java/org/jabref/model/TreeNode.java +++ b/src/main/java/org/jabref/model/TreeNode.java @@ -10,7 +10,6 @@ import javafx.collections.FXCollections; import javafx.collections.ObservableList; -import org.jabref.model.groups.GroupTreeNode; /** * Represents a node in a tree. diff --git a/src/main/java/org/jabref/model/database/BibDatabaseContext.java b/src/main/java/org/jabref/model/database/BibDatabaseContext.java index d59d12ce6d9..e0e9f2cb111 100644 --- a/src/main/java/org/jabref/model/database/BibDatabaseContext.java +++ b/src/main/java/org/jabref/model/database/BibDatabaseContext.java @@ -8,6 +8,7 @@ import java.util.Optional; import java.util.UUID; import java.util.stream.Collectors; + import javafx.collections.ObservableList; import org.jabref.architecture.AllowedToUseLogic; diff --git a/src/main/java/org/jabref/model/groups/AutomaticKeywordGroup.java b/src/main/java/org/jabref/model/groups/AutomaticKeywordGroup.java index d1c44ff748a..8f848df71cb 100644 --- a/src/main/java/org/jabref/model/groups/AutomaticKeywordGroup.java +++ b/src/main/java/org/jabref/model/groups/AutomaticKeywordGroup.java @@ -63,11 +63,10 @@ public int hashCode() { @Override public Set createSubgroups(BibEntry entry) { KeywordList keywordList = entry.getFieldAsKeywords(field, keywordDelimiter); - Set nodes = keywordList.stream() + return keywordList.stream() .filter(keyword -> StringUtil.isNotBlank(keyword.get())) .map(this::createGroup) .collect(Collectors.toSet()); - return nodes; } private GroupTreeNode createGroup(Keyword keywordChain) { diff --git a/src/main/java/org/jabref/model/groups/GroupTreeNode.java b/src/main/java/org/jabref/model/groups/GroupTreeNode.java index dc4f1942fb8..bb9188d051b 100644 --- a/src/main/java/org/jabref/model/groups/GroupTreeNode.java +++ b/src/main/java/org/jabref/model/groups/GroupTreeNode.java @@ -128,13 +128,14 @@ public boolean equals(Object o) { } GroupTreeNode that = (GroupTreeNode) o; return Objects.equals(group.get(), that.group.get()) && - Objects.equals(getChildren(), that.getChildren()); + getChildren() + .stream().map(GroupTreeNode::getGroup) + .allMatch(v -> that.getChildren().stream().map(GroupTreeNode::getGroup).anyMatch(v::equals)); } @Override public int hashCode() { - return group.hashCode(); - + return group.get().hashCode(); } /** diff --git a/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java b/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java index 18ff93df52e..42bbb6b7cdd 100644 --- a/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java +++ b/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java @@ -99,9 +99,9 @@ void treeOfAutomaticKeywordGroupIsCombined() { expectedA.addSubgroup(expectedGroupC); expectedA.addSubgroup(expectedGroupD); GroupNodeViewModel expectedE = getViewModelForGroup(expectedGroupE); - ObservableList expected = FXCollections.observableArrayList(expectedA.getGroupNode().getGroup(), expectedE.getGroupNode().getGroup()); + ObservableList expected = FXCollections.observableArrayList(expectedA.getGroupNode(), expectedE.getGroupNode()); - assertEquals(expected, groupViewModel.getGroupNode().getChildren().stream().map(GroupTreeNode::getGroup).collect(Collectors.toCollection(FXCollections::observableArrayList))); + assertEquals(expected, groupViewModel.getGroupNode().getChildren()); } @Test diff --git a/src/test/java/org/jabref/model/groups/AutomaticKeywordGroupTest.java b/src/test/java/org/jabref/model/groups/AutomaticKeywordGroupTest.java index a6a84698ff2..5c3e036d79e 100644 --- a/src/test/java/org/jabref/model/groups/AutomaticKeywordGroupTest.java +++ b/src/test/java/org/jabref/model/groups/AutomaticKeywordGroupTest.java @@ -2,6 +2,7 @@ import java.util.HashSet; import java.util.Set; +import java.util.stream.Collectors; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.field.StandardField; @@ -9,6 +10,7 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; class AutomaticKeywordGroupTest { @@ -19,7 +21,10 @@ void createSubgroupsForTwoKeywords() throws Exception { Set expected = createIncludingKeywordsSubgroup(); - assertEquals(expected, keywordsGroup.createSubgroups(entry)); + assertEquals( + expected.stream().map(GroupTreeNode::getGroup).collect(Collectors.toSet()), + keywordsGroup.createSubgroups(entry).stream().map(GroupTreeNode::getGroup).collect(Collectors.toSet()) + ); } @Test @@ -28,8 +33,10 @@ void createSubgroupsIgnoresEmptyKeyword() throws Exception { BibEntry entry = new BibEntry().withField(StandardField.KEYWORDS, "A, ,B"); Set expected = createIncludingKeywordsSubgroup(); + Set actual = keywordsGroup.createSubgroups(entry); - assertEquals(expected, keywordsGroup.createSubgroups(entry)); + assertEquals(actual, expected); + assertTrue(actual.containsAll(expected)); } private Set createIncludingKeywordsSubgroup() { From e4ec7758c5d999ed0dc70204152e19bd277dfaac Mon Sep 17 00:00:00 2001 From: m-peeler Date: Wed, 10 Jul 2024 17:01:28 -0400 Subject: [PATCH 14/24] Inverted conditional & removed unnecessary dependency --- .../gui/maintable/CreateGroupAction.java | 92 ++++++++++--------- .../gui/groups/GroupNodeViewModelTest.java | 1 - 2 files changed, 47 insertions(+), 46 deletions(-) diff --git a/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java b/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java index 4443b0fde84..d1ccd91c413 100644 --- a/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java +++ b/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java @@ -76,55 +76,57 @@ public void execute() { active = Optional.empty(); } - if (database.isPresent() && active.isPresent()) { - boolean isRoot = active.get().isRoot(); + if (database.isEmpty() || active.isEmpty()) { + dialogService.showWarningDialogAndWait(Localization.lang("Cannot create group"), Localization.lang("Cannot create group. Please create a library first.")); + return; + } + boolean isRoot = active.get().isRoot(); - final GroupTreeNode groupFallsUnder; - if (editGroup != null && editGroup.getParent().isPresent()) { - groupFallsUnder = editGroup.getParent().get(); - } else if (editGroup != null) { - groupFallsUnder = editGroup; - } else if (preferredAdditionLocation == PreferredGroupAdditionLocation.ADD_TO_ROOT) { - groupFallsUnder = active.get().getRoot(); - } else if (preferredAdditionLocation == PreferredGroupAdditionLocation.ADD_BESIDE - && !isRoot) { - groupFallsUnder = active.get().getParent().get(); - } else { - groupFallsUnder = active.get(); - } + final GroupTreeNode groupFallsUnder; + if (editGroup != null && editGroup.getParent().isPresent()) { + groupFallsUnder = editGroup.getParent().get(); + } else if (editGroup != null) { + groupFallsUnder = editGroup; + } else if (preferredAdditionLocation == PreferredGroupAdditionLocation.ADD_TO_ROOT) { + groupFallsUnder = active.get().getRoot(); + } else if (preferredAdditionLocation == PreferredGroupAdditionLocation.ADD_BESIDE + && !isRoot) { + groupFallsUnder = active.get().getParent().isPresent() + ? active.get().getParent().get() + : active.get(); + } else { + groupFallsUnder = active.get(); + } - Optional newGroup = dialogService.showCustomDialogAndWait( - new GroupDialogView( - database.get(), - groupFallsUnder, - editGroup != null ? editGroup.getGroup() : null, - isRoot ? GroupDialogHeader.GROUP : GroupDialogHeader.SUBGROUP, - stateManager.getSelectedEntries(), - preferSelectedEntries - )); + Optional newGroup = dialogService.showCustomDialogAndWait( + new GroupDialogView( + database.get(), + groupFallsUnder, + editGroup != null ? editGroup.getGroup() : null, + isRoot ? GroupDialogHeader.GROUP : GroupDialogHeader.SUBGROUP, + stateManager.getSelectedEntries(), + preferSelectedEntries + )); - newGroup.ifPresent(group -> { - GroupTreeNode newSubgroup; - if (editGroup != null) { - Optional editedGroup = groupEditActions(editGroup, group, database.get()); - if (editedGroup.isEmpty()) { - return; - } - newSubgroup = editedGroup.get(); - dialogService.notify(Localization.lang("Modified group \"%0\".", newSubgroup.getName())); - } else { - newSubgroup = groupFallsUnder.addSubgroup(group); - dialogService.notify(Localization.lang("Added group \"%0\".", newSubgroup.getName())); + newGroup.ifPresent(group -> { + GroupTreeNode newSubgroup; + if (editGroup != null) { + Optional editedGroup = groupEditActions(editGroup, group, database.get()); + if (editedGroup.isEmpty()) { + return; } - stateManager.getSelectedGroups(database.get()).setAll(newSubgroup); - // TODO: Add undo - // UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(parent, new GroupTreeNodeViewModel(newGroupNode), UndoableAddOrRemoveGroup.ADD_NODE); - // panel.getUndoManager().addEdit(undo); - writeGroupChangesToMetaData(database.get(), newSubgroup); - }); - } else { - dialogService.showWarningDialogAndWait(Localization.lang("Cannot create group"), Localization.lang("Cannot create group. Please create a library first.")); - } + newSubgroup = editedGroup.get(); + dialogService.notify(Localization.lang("Modified group \"%0\".", newSubgroup.getName())); + } else { + newSubgroup = groupFallsUnder.addSubgroup(group); + dialogService.notify(Localization.lang("Added group \"%0\".", newSubgroup.getName())); + } + stateManager.getSelectedGroups(database.get()).setAll(newSubgroup); + // TODO: Add undo + // UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(parent, new GroupTreeNodeViewModel(newGroupNode), UndoableAddOrRemoveGroup.ADD_NODE); + // panel.getUndoManager().addEdit(undo); + writeGroupChangesToMetaData(database.get(), newSubgroup); + }); } private Optional groupEditActions(GroupTreeNode node, AbstractGroup newGroup, BibDatabaseContext database) { diff --git a/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java b/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java index 42bbb6b7cdd..a7a15c3f667 100644 --- a/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java +++ b/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java @@ -1,7 +1,6 @@ package org.jabref.gui.groups; import java.util.Arrays; -import java.util.stream.Collectors; import javafx.collections.FXCollections; import javafx.collections.ObservableList; From e2ad78457c1dcd6fee7879a5efa460b71910d0be Mon Sep 17 00:00:00 2001 From: m-peeler Date: Wed, 10 Jul 2024 19:15:11 -0400 Subject: [PATCH 15/24] Added "Add Subgroup from Selection" to groups context menu. Fixed style issues, added new context menu option, fixed bug with update order when selecting a new active group after creating it --- .../org/jabref/gui/actions/ActionHelper.java | 25 ++++++ .../jabref/gui/actions/StandardActions.java | 4 +- .../org/jabref/gui/groups/GroupTreeView.java | 5 +- .../jabref/gui/groups/GroupTreeViewModel.java | 2 +- .../gui/maintable/CreateGroupAction.java | 89 +++++++++++-------- .../jabref/gui/maintable/RightClickMenu.java | 4 +- 6 files changed, 88 insertions(+), 41 deletions(-) diff --git a/src/main/java/org/jabref/gui/actions/ActionHelper.java b/src/main/java/org/jabref/gui/actions/ActionHelper.java index 20376f2f3e2..4cd171b3dce 100644 --- a/src/main/java/org/jabref/gui/actions/ActionHelper.java +++ b/src/main/java/org/jabref/gui/actions/ActionHelper.java @@ -4,10 +4,12 @@ import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.stream.Collectors; import javafx.beans.binding.Binding; import javafx.beans.binding.Bindings; import javafx.beans.binding.BooleanExpression; +import javafx.collections.FXCollections; import javafx.collections.ObservableList; import javafx.scene.control.TabPane; @@ -18,6 +20,7 @@ import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.LinkedFile; import org.jabref.model.entry.field.Field; +import org.jabref.model.groups.GroupTreeNode; import org.jabref.preferences.PreferencesService; import com.tobiasdiez.easybind.EasyBind; @@ -43,6 +46,28 @@ public static BooleanExpression needsStudyDatabase(StateManager stateManager) { return BooleanExpression.booleanExpression(binding); } + // Makes sure there is at least one group selected, and if there are multiple groups selected + // all have the same parent node. + public static BooleanExpression needsSelectedGroupsShareParent(StateManager stateManager) { + if (stateManager.activeGroupProperty().isEmpty()) { + return Bindings.size(stateManager.activeGroupProperty()).isEqualTo(1); + } + return Bindings.size(stateManager.activeGroupProperty()).isEqualTo(1) + .or(Bindings.size(stateManager.activeGroupProperty()).greaterThan(1).and( + Bindings.size( + (ObservableList) stateManager + .activeGroupProperty() + .stream() + .filter(stateManager.activeGroupProperty().get().getFirst()::equals) + .collect(Collectors.toCollection(FXCollections::observableArrayList)) + ).isEqualTo(Bindings.size(stateManager.activeGroupProperty()))) + ); + } + + public static BooleanExpression needsNotMultipleGroupsSelected(StateManager stateManager) { + return Bindings.size(stateManager.activeGroupProperty()).lessThan(2); + } + public static BooleanExpression needsEntriesSelected(StateManager stateManager) { return Bindings.isNotEmpty(stateManager.getSelectedEntries()); } diff --git a/src/main/java/org/jabref/gui/actions/StandardActions.java b/src/main/java/org/jabref/gui/actions/StandardActions.java index e7deb3abe38..aa1a610f22f 100644 --- a/src/main/java/org/jabref/gui/actions/StandardActions.java +++ b/src/main/java/org/jabref/gui/actions/StandardActions.java @@ -36,8 +36,8 @@ public enum StandardActions implements Action { EXTRACT_FILE_REFERENCES_OFFLINE(Localization.lang("Extract references from file (offline)"), IconTheme.JabRefIcons.FILE_STAR), OPEN_URL(Localization.lang("Open URL or DOI"), IconTheme.JabRefIcons.WWW, KeyBinding.OPEN_URL_OR_DOI), SEARCH_SHORTSCIENCE(Localization.lang("Search ShortScience")), - CREATE_GROUP_FROM_SELECTION(Localization.lang("Create group from selection")), - CREATE_SUBGROUP_FROM_SELECTION(Localization.lang("Create subgroup from selection")), + ADD_GROUP_FROM_SELECTION(Localization.lang("Add group from selection")), + ADD_SUBGROUP_FROM_SELECTION(Localization.lang("Add subgroup from selection")), MERGE_WITH_FETCHED_ENTRY(Localization.lang("Get bibliographic data from %0", "DOI/ISBN/...")), ATTACH_FILE(Localization.lang("Attach file"), IconTheme.JabRefIcons.ATTACH_FILE), ATTACH_FILE_FROM_URL(Localization.lang("Attach file from URL"), IconTheme.JabRefIcons.DOWNLOAD_FILE), diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeView.java b/src/main/java/org/jabref/gui/groups/GroupTreeView.java index f6f3b6c4431..324fbc89eca 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeView.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeView.java @@ -50,6 +50,8 @@ import org.jabref.gui.actions.ActionFactory; import org.jabref.gui.actions.SimpleCommand; import org.jabref.gui.actions.StandardActions; +import org.jabref.gui.maintable.CreateGroupAction; +import org.jabref.gui.maintable.PreferredGroupAdditionLocation; import org.jabref.gui.search.SearchTextField; import org.jabref.gui.util.BindingsHelper; import org.jabref.gui.util.ControlHelper; @@ -408,7 +410,7 @@ private void handleOnDragOver(TreeTableRow row, GroupNodeVie } private void updateSelection(List newActiveGroups, BibDatabaseContext database) { - if ((newActiveGroups == null)) { + if (newActiveGroups == null) { stateManager.setSelectedGroups(database, Collections.singletonList(viewModel.rootGroup().getGroupNode())); } else { List list = newActiveGroups.stream() @@ -580,6 +582,7 @@ private ContextMenu createContextMenuForGroup(GroupNodeViewModel group) { removeGroup, new SeparatorMenuItem(), factory.createMenuItem(StandardActions.GROUP_SUBGROUP_ADD, new ContextAction(StandardActions.GROUP_SUBGROUP_ADD, group)), + factory.createMenuItem(StandardActions.ADD_SUBGROUP_FROM_SELECTION, new CreateGroupAction(dialogService, stateManager, true, PreferredGroupAdditionLocation.ADD_BELOW, null)), factory.createMenuItem(StandardActions.GROUP_SUBGROUP_REMOVE, new ContextAction(StandardActions.GROUP_SUBGROUP_REMOVE, group)), factory.createMenuItem(StandardActions.GROUP_SUBGROUP_SORT, new ContextAction(StandardActions.GROUP_SUBGROUP_SORT, group)), factory.createMenuItem(StandardActions.GROUP_SUBGROUP_SORT_REVERSE, new ContextAction(StandardActions.GROUP_SUBGROUP_SORT_REVERSE, group)), diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java index d40177bea9e..89702983ff4 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java @@ -109,7 +109,7 @@ private void onActiveGroupChange(ListChangeListener.Change { - if ((newValue == null)) { + if (newValue == null) { selectedGroups.clear(); } else { selectedGroups.setAll(newValue.getList()); diff --git a/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java b/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java index d1ccd91c413..37b21db8706 100644 --- a/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java +++ b/src/main/java/org/jabref/gui/maintable/CreateGroupAction.java @@ -1,5 +1,6 @@ package org.jabref.gui.maintable; +import java.util.Collections; import java.util.Objects; import java.util.Optional; @@ -27,14 +28,18 @@ import org.jspecify.annotations.Nullable; import static org.jabref.gui.actions.ActionHelper.needsEntriesSelected; +import static org.jabref.gui.actions.ActionHelper.needsNotMultipleGroupsSelected; +import static org.jabref.gui.actions.ActionHelper.needsSelectedGroupsShareParent; public class CreateGroupAction extends SimpleCommand { private final DialogService dialogService; private final StateManager stateManager; private final boolean preferSelectedEntries; - private final PreferredGroupAdditionLocation preferredAdditionLocation; private final GroupTreeNode editGroup; + private final GroupTreeNode prospectiveParent; + private final Optional database; + private final Optional active; public CreateGroupAction( DialogService dialogService, @@ -46,27 +51,9 @@ public CreateGroupAction( this.dialogService = dialogService; this.stateManager = stateManager; this.preferSelectedEntries = preferSelectedEntries; - this.preferredAdditionLocation = preferredAdditionLocation; this.editGroup = editGroup; - this.executable.bind(needsEntriesSelected(stateManager)); - } - - public CreateGroupAction( - DialogService dialogService, - StateManager stateManager - ) { - this(dialogService, stateManager, false, PreferredGroupAdditionLocation.ADD_BESIDE, null); - } - - public void writeGroupChangesToMetaData(BibDatabaseContext database, GroupTreeNode newGroup) { - database.getMetaData().setGroups(newGroup.getRoot()); - } - - @Override - public void execute() { - Optional database = stateManager.getActiveDatabase(); - Optional active; + database = stateManager.getActiveDatabase(); if (!stateManager.activeGroupProperty().isEmpty()) { active = Optional.of(stateManager.activeGroupProperty().getFirst()); @@ -76,34 +63,65 @@ public void execute() { active = Optional.empty(); } - if (database.isEmpty() || active.isEmpty()) { - dialogService.showWarningDialogAndWait(Localization.lang("Cannot create group"), Localization.lang("Cannot create group. Please create a library first.")); - return; + if (active.isEmpty()) { + throw new IllegalStateException("ERROR: No parent findable which this group could be added to."); } + boolean isRoot = active.get().isRoot(); - final GroupTreeNode groupFallsUnder; if (editGroup != null && editGroup.getParent().isPresent()) { - groupFallsUnder = editGroup.getParent().get(); + prospectiveParent = editGroup.getParent().get(); } else if (editGroup != null) { - groupFallsUnder = editGroup; + prospectiveParent = editGroup; } else if (preferredAdditionLocation == PreferredGroupAdditionLocation.ADD_TO_ROOT) { - groupFallsUnder = active.get().getRoot(); + prospectiveParent = active.get().getRoot(); } else if (preferredAdditionLocation == PreferredGroupAdditionLocation.ADD_BESIDE && !isRoot) { - groupFallsUnder = active.get().getParent().isPresent() + prospectiveParent = active.get().getParent().isPresent() ? active.get().getParent().get() : active.get(); } else { - groupFallsUnder = active.get(); + prospectiveParent = active.get(); + } + + switch (preferredAdditionLocation) { + case ADD_BESIDE: + this.executable.bind(needsEntriesSelected(stateManager).and(needsSelectedGroupsShareParent(stateManager))); + break; + case ADD_BELOW: + this.executable.bind(needsEntriesSelected(stateManager).and(needsNotMultipleGroupsSelected(stateManager))); + break; + case ADD_TO_ROOT: + default: + this.executable.bind(needsEntriesSelected(stateManager)); + break; + } + } + + public CreateGroupAction( + DialogService dialogService, + StateManager stateManager + ) { + this(dialogService, stateManager, false, PreferredGroupAdditionLocation.ADD_BESIDE, null); + } + + public void writeGroupChangesToMetaData(BibDatabaseContext database, GroupTreeNode newGroup) { + database.getMetaData().setGroups(newGroup.getRoot()); + } + + @Override + public void execute() { + if (database.isEmpty() || prospectiveParent == null) { + dialogService.showWarningDialogAndWait(Localization.lang("Cannot create group"), Localization.lang("Cannot create group. Please create a library first.")); + return; } Optional newGroup = dialogService.showCustomDialogAndWait( new GroupDialogView( database.get(), - groupFallsUnder, + prospectiveParent, editGroup != null ? editGroup.getGroup() : null, - isRoot ? GroupDialogHeader.GROUP : GroupDialogHeader.SUBGROUP, + prospectiveParent.isRoot() ? GroupDialogHeader.GROUP : GroupDialogHeader.SUBGROUP, stateManager.getSelectedEntries(), preferSelectedEntries )); @@ -118,14 +136,15 @@ public void execute() { newSubgroup = editedGroup.get(); dialogService.notify(Localization.lang("Modified group \"%0\".", newSubgroup.getName())); } else { - newSubgroup = groupFallsUnder.addSubgroup(group); + newSubgroup = prospectiveParent.addSubgroup(group); dialogService.notify(Localization.lang("Added group \"%0\".", newSubgroup.getName())); } - stateManager.getSelectedGroups(database.get()).setAll(newSubgroup); + // TODO: Add undo // UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(parent, new GroupTreeNodeViewModel(newGroupNode), UndoableAddOrRemoveGroup.ADD_NODE); // panel.getUndoManager().addEdit(undo); writeGroupChangesToMetaData(database.get(), newSubgroup); + stateManager.setSelectedGroups(database.get(), Collections.singletonList(newSubgroup)); }); } @@ -161,7 +180,7 @@ private Optional groupEditActions(GroupTreeNode node, AbstractGro removePreviousAssignments = (node.getGroup() instanceof ExplicitGroup) && (newGroup instanceof ExplicitGroup) && nameIsUnique(oldGroup, newGroup, database); - keepPreviousAssignments = (reassignmentResponse.get().getButtonData() == ButtonBar.ButtonData.YES); + keepPreviousAssignments = reassignmentResponse.get().getButtonData() == ButtonBar.ButtonData.YES; } // TODO: Add undo // Store undo information. @@ -248,7 +267,7 @@ private boolean nameIsUnique(AbstractGroup oldGroup, AbstractGroup newGroup, Bib // we need to check the old name for duplicates. If the new group name occurs more than once, it won't matter groupsWithSameName = databaseRootGroup.get().findChildrenSatisfying(g -> g.getName().equals(oldGroup.getName())).size(); } - return !(groupsWithSameName >= 2); + return groupsWithSameName < 2; } private Optional showConfirmationPanel(boolean isNowWordKeywordGroup) { diff --git a/src/main/java/org/jabref/gui/maintable/RightClickMenu.java b/src/main/java/org/jabref/gui/maintable/RightClickMenu.java index 803e7265e56..6044f6cbb73 100644 --- a/src/main/java/org/jabref/gui/maintable/RightClickMenu.java +++ b/src/main/java/org/jabref/gui/maintable/RightClickMenu.java @@ -79,8 +79,8 @@ public static ContextMenu create(BibEntryTableViewModel entry, // the button's name changes based on whether we are in the root group // or a subgroup. stateManager.activeGroupProperty().isEmpty() || stateManager.activeGroupProperty().getFirst().getParent().isEmpty() - ? StandardActions.CREATE_GROUP_FROM_SELECTION - : StandardActions.CREATE_SUBGROUP_FROM_SELECTION, + ? StandardActions.ADD_GROUP_FROM_SELECTION + : StandardActions.ADD_SUBGROUP_FROM_SELECTION, new CreateGroupAction(dialogService, stateManager, true, PreferredGroupAdditionLocation.ADD_BELOW, null)), new SeparatorMenuItem(), From 85d42b921e984b75e098a1ce6c7693b1478b6f89 Mon Sep 17 00:00:00 2001 From: m-peeler Date: Wed, 10 Jul 2024 19:25:15 -0400 Subject: [PATCH 16/24] Fixed text comparison issue --- src/main/resources/l10n/JabRef_en.properties | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index 1a4149ea99c..c6a11a5f1c7 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -2650,5 +2650,5 @@ Warning\:\ Failed\ to\ check\ if\ the\ directory\ is\ empty.=Warning: Failed to Warning\:\ The\ selected\ directory\ is\ not\ a\ valid\ directory.=Warning: The selected directory is not a valid directory. Current\ selection=Current selection Group\ all\ entries\ currently\ selected=Group all entries currently selected -Create\ group\ from\ selection=Create group from selection -Create\ subgroup\ from\ selection=Create subgroup from selection +Add\ group\ from\ selection=Add group from selection +Add\ subgroup\ from\ selection=Add subgroup from selection From 00de24bb9719d5e89838a8a66d060870f5f0c814 Mon Sep 17 00:00:00 2001 From: m-peeler Date: Thu, 11 Jul 2024 02:51:58 -0400 Subject: [PATCH 17/24] Fixed bad bindings --- .../java/org/jabref/gui/StateManager.java | 7 ++--- .../org/jabref/gui/actions/ActionHelper.java | 29 ++++++++++--------- .../jabref/gui/maintable/RightClickMenu.java | 9 +----- .../org/jabref/gui/util/BindingsHelper.java | 2 ++ 4 files changed, 22 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/jabref/gui/StateManager.java b/src/main/java/org/jabref/gui/StateManager.java index 0c297da2672..cd67bdcbd25 100644 --- a/src/main/java/org/jabref/gui/StateManager.java +++ b/src/main/java/org/jabref/gui/StateManager.java @@ -75,13 +75,12 @@ public class StateManager { private final ObservableList searchHistory = FXCollections.observableArrayList(); public StateManager() { - BibDatabaseContext nullContext = new BibDatabaseContext(); - selectedGroups.put(nullContext.getUid(), FXCollections.observableArrayList()); activeGroups.bind(Bindings.valueAt( selectedGroups, activeDatabase - .orElseOpt(nullContext) - .map(BibDatabaseContext::getUid))); + .orElseOpt(null) + .map(BibDatabaseContext::getUid)) + .orElse(FXCollections.observableArrayList())); } public ObservableList getVisibleSidePaneComponents() { diff --git a/src/main/java/org/jabref/gui/actions/ActionHelper.java b/src/main/java/org/jabref/gui/actions/ActionHelper.java index 4cd171b3dce..1132cae13bf 100644 --- a/src/main/java/org/jabref/gui/actions/ActionHelper.java +++ b/src/main/java/org/jabref/gui/actions/ActionHelper.java @@ -3,17 +3,22 @@ import java.nio.file.Path; import java.util.Collections; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; import javafx.beans.binding.Binding; import javafx.beans.binding.Bindings; +import javafx.beans.binding.BooleanBinding; import javafx.beans.binding.BooleanExpression; +import javafx.beans.property.SimpleBooleanProperty; +import javafx.beans.value.ObservableBooleanValue; import javafx.collections.FXCollections; import javafx.collections.ObservableList; import javafx.scene.control.TabPane; import org.jabref.gui.StateManager; +import org.jabref.gui.util.BindingsHelper; import org.jabref.logic.shared.DatabaseLocation; import org.jabref.logic.util.io.FileUtil; import org.jabref.model.database.BibDatabaseContext; @@ -49,19 +54,17 @@ public static BooleanExpression needsStudyDatabase(StateManager stateManager) { // Makes sure there is at least one group selected, and if there are multiple groups selected // all have the same parent node. public static BooleanExpression needsSelectedGroupsShareParent(StateManager stateManager) { - if (stateManager.activeGroupProperty().isEmpty()) { - return Bindings.size(stateManager.activeGroupProperty()).isEqualTo(1); - } - return Bindings.size(stateManager.activeGroupProperty()).isEqualTo(1) - .or(Bindings.size(stateManager.activeGroupProperty()).greaterThan(1).and( - Bindings.size( - (ObservableList) stateManager - .activeGroupProperty() - .stream() - .filter(stateManager.activeGroupProperty().get().getFirst()::equals) - .collect(Collectors.toCollection(FXCollections::observableArrayList)) - ).isEqualTo(Bindings.size(stateManager.activeGroupProperty()))) - ); + return Bindings.size(stateManager.activeGroupProperty()).isEqualTo(1) // Is single element OR + .or(Bindings.size(stateManager.activeGroupProperty()).greaterThan(1) // (Is multiple elements AND + .and(Bindings.createBooleanBinding( + () -> stateManager.activeGroupProperty().stream() + .allMatch(val -> // The parent of the first element + stateManager.activeGroupProperty() // is equal to the parent of all + .getFirst() // other elements + .getParent() // (including if they are null)) + .map(node -> node.equals(val.getParent().get())) + .orElseGet(() -> val.getParent().isEmpty())), + stateManager.activeGroupProperty()))); } public static BooleanExpression needsNotMultipleGroupsSelected(StateManager stateManager) { diff --git a/src/main/java/org/jabref/gui/maintable/RightClickMenu.java b/src/main/java/org/jabref/gui/maintable/RightClickMenu.java index 6044f6cbb73..9bc520ca06d 100644 --- a/src/main/java/org/jabref/gui/maintable/RightClickMenu.java +++ b/src/main/java/org/jabref/gui/maintable/RightClickMenu.java @@ -74,14 +74,7 @@ public static ContextMenu create(BibEntryTableViewModel entry, SpecialFieldMenuItemFactory.getSpecialFieldSingleItem(SpecialField.PRINTED, factory, () -> libraryTab, dialogService, preferencesService, undoManager, stateManager), SpecialFieldMenuItemFactory.createSpecialFieldMenu(SpecialField.PRIORITY, factory, () -> libraryTab, dialogService, preferencesService, undoManager, stateManager), SpecialFieldMenuItemFactory.createSpecialFieldMenu(SpecialField.READ_STATUS, factory, () -> libraryTab, dialogService, preferencesService, undoManager, stateManager), - factory.createMenuItem( - // The new group is placed inside the currently active group, so - // the button's name changes based on whether we are in the root group - // or a subgroup. - stateManager.activeGroupProperty().isEmpty() || stateManager.activeGroupProperty().getFirst().getParent().isEmpty() - ? StandardActions.ADD_GROUP_FROM_SELECTION - : StandardActions.ADD_SUBGROUP_FROM_SELECTION, - new CreateGroupAction(dialogService, stateManager, true, PreferredGroupAdditionLocation.ADD_BELOW, null)), + factory.createMenuItem(StandardActions.ADD_SUBGROUP_FROM_SELECTION, new CreateGroupAction(dialogService, stateManager, true, PreferredGroupAdditionLocation.ADD_BELOW, null)), new SeparatorMenuItem(), diff --git a/src/main/java/org/jabref/gui/util/BindingsHelper.java b/src/main/java/org/jabref/gui/util/BindingsHelper.java index 48cef619852..eb69772bff1 100644 --- a/src/main/java/org/jabref/gui/util/BindingsHelper.java +++ b/src/main/java/org/jabref/gui/util/BindingsHelper.java @@ -10,7 +10,9 @@ import javafx.beans.binding.StringBinding; import javafx.beans.property.ListProperty; import javafx.beans.property.Property; +import javafx.beans.property.SimpleObjectProperty; import javafx.beans.value.ChangeListener; +import javafx.beans.value.ObservableBooleanValue; import javafx.beans.value.ObservableValue; import javafx.collections.FXCollections; import javafx.collections.ListChangeListener; From a1dfe7531db7178495831f4780ac36c961310ea1 Mon Sep 17 00:00:00 2001 From: m-peeler Date: Thu, 11 Jul 2024 03:22:50 -0400 Subject: [PATCH 18/24] Removed accidential imports --- src/main/java/org/jabref/gui/actions/ActionHelper.java | 8 -------- src/main/java/org/jabref/gui/util/BindingsHelper.java | 2 -- 2 files changed, 10 deletions(-) diff --git a/src/main/java/org/jabref/gui/actions/ActionHelper.java b/src/main/java/org/jabref/gui/actions/ActionHelper.java index 1132cae13bf..35ce8508526 100644 --- a/src/main/java/org/jabref/gui/actions/ActionHelper.java +++ b/src/main/java/org/jabref/gui/actions/ActionHelper.java @@ -3,29 +3,21 @@ import java.nio.file.Path; import java.util.Collections; import java.util.List; -import java.util.Objects; import java.util.Optional; -import java.util.stream.Collectors; import javafx.beans.binding.Binding; import javafx.beans.binding.Bindings; -import javafx.beans.binding.BooleanBinding; import javafx.beans.binding.BooleanExpression; -import javafx.beans.property.SimpleBooleanProperty; -import javafx.beans.value.ObservableBooleanValue; -import javafx.collections.FXCollections; import javafx.collections.ObservableList; import javafx.scene.control.TabPane; import org.jabref.gui.StateManager; -import org.jabref.gui.util.BindingsHelper; import org.jabref.logic.shared.DatabaseLocation; import org.jabref.logic.util.io.FileUtil; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.LinkedFile; import org.jabref.model.entry.field.Field; -import org.jabref.model.groups.GroupTreeNode; import org.jabref.preferences.PreferencesService; import com.tobiasdiez.easybind.EasyBind; diff --git a/src/main/java/org/jabref/gui/util/BindingsHelper.java b/src/main/java/org/jabref/gui/util/BindingsHelper.java index eb69772bff1..48cef619852 100644 --- a/src/main/java/org/jabref/gui/util/BindingsHelper.java +++ b/src/main/java/org/jabref/gui/util/BindingsHelper.java @@ -10,9 +10,7 @@ import javafx.beans.binding.StringBinding; import javafx.beans.property.ListProperty; import javafx.beans.property.Property; -import javafx.beans.property.SimpleObjectProperty; import javafx.beans.value.ChangeListener; -import javafx.beans.value.ObservableBooleanValue; import javafx.beans.value.ObservableValue; import javafx.collections.FXCollections; import javafx.collections.ListChangeListener; From 77f927bce5263435f02364b9051730792158e95f Mon Sep 17 00:00:00 2001 From: m-peeler Date: Thu, 11 Jul 2024 18:31:02 -0400 Subject: [PATCH 19/24] Reverted Main Table Context-Menu change; Added 'include selected' checkbox & preference Revered changes to context menu on the main table. Removed 'Current selection' radio button in `GroupDialog` and replaced with a 'Include currently selected entries' checkbox, liked both to a parameter in `GroupDialogViewModel` and a preference added to `GroupPreferences`. Edited preference menu to include this new preference. Streamlined `CreateGroupAction` to better fit these changes. Fixed bug where dragging new empty article entries into a new group wouldn't increment the group count. --- .../org/jabref/gui/actions/ActionHelper.java | 20 ------ .../jabref/gui/actions/StandardActions.java | 1 - .../jabref/gui/exporter/ExportCommand.java | 7 ++- .../org/jabref/gui/groups/GroupDialog.fxml | 9 +-- .../jabref/gui/groups/GroupDialogView.java | 25 +++----- .../gui/groups/GroupDialogViewModel.java | 53 ++++++---------- .../jabref/gui/groups/GroupNodeViewModel.java | 7 ++- .../org/jabref/gui/groups/GroupTreeView.java | 8 +-- .../jabref/gui/groups/GroupTreeViewModel.java | 24 ++++---- .../jabref/gui/groups/GroupsPreferences.java | 15 +++++ .../gui/maintable/CreateGroupAction.java | 61 +++---------------- .../PreferredGroupAdditionLocation.java | 7 --- .../jabref/gui/maintable/RightClickMenu.java | 1 - .../org/jabref/gui/maintable/Selection.java | 6 ++ .../gui/preferences/groups/GroupsTab.fxml | 2 + .../gui/preferences/groups/GroupsTab.java | 3 +- .../groups/GroupsTabViewModel.java | 7 +++ .../jabref/model/groups/AbstractGroup.java | 10 --- .../jabref/model/groups/GroupTreeNode.java | 7 ++- .../jabref/preferences/JabRefPreferences.java | 4 ++ src/main/resources/l10n/JabRef_en.properties | 5 +- .../gui/groups/GroupNodeViewModelTest.java | 1 + .../gui/groups/GroupTreeViewModelTest.java | 1 + 23 files changed, 115 insertions(+), 169 deletions(-) delete mode 100644 src/main/java/org/jabref/gui/maintable/PreferredGroupAdditionLocation.java create mode 100644 src/main/java/org/jabref/gui/maintable/Selection.java diff --git a/src/main/java/org/jabref/gui/actions/ActionHelper.java b/src/main/java/org/jabref/gui/actions/ActionHelper.java index 35ce8508526..20376f2f3e2 100644 --- a/src/main/java/org/jabref/gui/actions/ActionHelper.java +++ b/src/main/java/org/jabref/gui/actions/ActionHelper.java @@ -43,26 +43,6 @@ public static BooleanExpression needsStudyDatabase(StateManager stateManager) { return BooleanExpression.booleanExpression(binding); } - // Makes sure there is at least one group selected, and if there are multiple groups selected - // all have the same parent node. - public static BooleanExpression needsSelectedGroupsShareParent(StateManager stateManager) { - return Bindings.size(stateManager.activeGroupProperty()).isEqualTo(1) // Is single element OR - .or(Bindings.size(stateManager.activeGroupProperty()).greaterThan(1) // (Is multiple elements AND - .and(Bindings.createBooleanBinding( - () -> stateManager.activeGroupProperty().stream() - .allMatch(val -> // The parent of the first element - stateManager.activeGroupProperty() // is equal to the parent of all - .getFirst() // other elements - .getParent() // (including if they are null)) - .map(node -> node.equals(val.getParent().get())) - .orElseGet(() -> val.getParent().isEmpty())), - stateManager.activeGroupProperty()))); - } - - public static BooleanExpression needsNotMultipleGroupsSelected(StateManager stateManager) { - return Bindings.size(stateManager.activeGroupProperty()).lessThan(2); - } - public static BooleanExpression needsEntriesSelected(StateManager stateManager) { return Bindings.isNotEmpty(stateManager.getSelectedEntries()); } diff --git a/src/main/java/org/jabref/gui/actions/StandardActions.java b/src/main/java/org/jabref/gui/actions/StandardActions.java index aa1a610f22f..9d9ec20fbd5 100644 --- a/src/main/java/org/jabref/gui/actions/StandardActions.java +++ b/src/main/java/org/jabref/gui/actions/StandardActions.java @@ -36,7 +36,6 @@ public enum StandardActions implements Action { EXTRACT_FILE_REFERENCES_OFFLINE(Localization.lang("Extract references from file (offline)"), IconTheme.JabRefIcons.FILE_STAR), OPEN_URL(Localization.lang("Open URL or DOI"), IconTheme.JabRefIcons.WWW, KeyBinding.OPEN_URL_OR_DOI), SEARCH_SHORTSCIENCE(Localization.lang("Search ShortScience")), - ADD_GROUP_FROM_SELECTION(Localization.lang("Add group from selection")), ADD_SUBGROUP_FROM_SELECTION(Localization.lang("Add subgroup from selection")), MERGE_WITH_FETCHED_ENTRY(Localization.lang("Get bibliographic data from %0", "DOI/ISBN/...")), ATTACH_FILE(Localization.lang("Attach file"), IconTheme.JabRefIcons.ATTACH_FILE), diff --git a/src/main/java/org/jabref/gui/exporter/ExportCommand.java b/src/main/java/org/jabref/gui/exporter/ExportCommand.java index 814d639199a..891179de21c 100644 --- a/src/main/java/org/jabref/gui/exporter/ExportCommand.java +++ b/src/main/java/org/jabref/gui/exporter/ExportCommand.java @@ -2,12 +2,13 @@ import java.io.IOException; import java.nio.file.Path; +import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.function.Supplier; import java.util.stream.Collectors; +import java.util.stream.Stream; -import javafx.collections.FXCollections; import javafx.stage.FileChooser; import javafx.util.Duration; @@ -111,7 +112,9 @@ private void export(Path file, FileChooser.ExtensionFilter selectedExtensionFilt // All entries entries = stateManager.getActiveDatabase() .map(BibDatabaseContext::getEntries) - .orElse(FXCollections.emptyObservableList()); + .map(List::stream) + .map(Stream::toList) + .orElse(Collections.emptyList()); } List fileDirForDatabase = stateManager.getActiveDatabase() diff --git a/src/main/java/org/jabref/gui/groups/GroupDialog.fxml b/src/main/java/org/jabref/gui/groups/GroupDialog.fxml index cb26bcf91e4..553fc1aa8a4 100644 --- a/src/main/java/org/jabref/gui/groups/GroupDialog.fxml +++ b/src/main/java/org/jabref/gui/groups/GroupDialog.fxml @@ -99,15 +99,12 @@ - - - - - + + +