Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Right click create group #11476

Closed
wants to merge 38 commits into from
Closed
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
910c857
Auto-Select Newly Created Group
m-peeler Jul 5, 2024
cbc0d7d
Update CHANGELOG.md
m-peeler Jul 5, 2024
f126974
Implemented "Current selection" group option
m-peeler Jul 5, 2024
e4c2281
Addressed Style and Property Name Errors
m-peeler Jul 5, 2024
9b4d2db
Created 'Create Group From Selection' action
m-peeler Jul 5, 2024
9631067
Merge branch 'focus-group-on-creation' into right-click-create-group
m-peeler Jul 5, 2024
d4222a7
Merge branch 'new-group-from-selected' into right-click-create-group
m-peeler Jul 5, 2024
faca154
Worked to implement right-click-to-make-group functionality
m-peeler Jul 7, 2024
ca5e69f
Merge branch 'focus-group-on-creation'
m-peeler Jul 7, 2024
3a6037e
Feature added: Right Click on entries to create group from selection
m-peeler Jul 9, 2024
8dd474a
Update CHANGELOG.md
m-peeler Jul 9, 2024
12dedd4
Merge branch 'main' into right-click-create-group
m-peeler Jul 9, 2024
2c87cda
Merge branch 'main' of https://github.com/JabRef/jabref
m-peeler Jul 9, 2024
f3011ed
Merge branch 'main' into right-click-create-group
m-peeler Jul 9, 2024
53f1544
Update CHANGELOG.md
m-peeler Jul 9, 2024
ec9b081
Update CHANGELOG.md
m-peeler Jul 10, 2024
aabb4a3
Fixed formatting issues & addressed test failures
m-peeler Jul 10, 2024
f654aba
Merge branch 'right-click-create-group' of https://github.com/m-peele…
m-peeler Jul 10, 2024
d772aaa
Fixed unnecessary module
m-peeler Jul 10, 2024
d3e1f1e
Style & Bug Fixes
m-peeler Jul 10, 2024
e4ec775
Inverted conditional & removed unnecessary dependency
m-peeler Jul 10, 2024
fd088d1
Merge branch 'main' into right-click-create-group
Siedlerchr Jul 10, 2024
e2ad784
Added "Add Subgroup from Selection" to groups context menu.
m-peeler Jul 10, 2024
cf33b73
Merge branch 'right-click-create-group' of https://github.com/m-peele…
m-peeler Jul 10, 2024
85d42b9
Fixed text comparison issue
m-peeler Jul 10, 2024
ea49e46
Merge branch 'main' of https://github.com/JabRef/jabref
m-peeler Jul 11, 2024
00de24b
Fixed bad bindings
m-peeler Jul 11, 2024
a1dfe75
Removed accidential imports
m-peeler Jul 11, 2024
77f927b
Reverted Main Table Context-Menu change; Added 'include selected' che…
m-peeler Jul 11, 2024
f80e515
Discard changes to src/main/java/org/jabref/gui/exporter/ExportComman…
koppor Jul 12, 2024
c5241f0
Bug fixes, variable ordering & unnecessary change reversions
m-peeler Jul 13, 2024
bba0522
Merge branch 'right-click-create-group' of https://github.com/m-peele…
m-peeler Jul 13, 2024
1912281
Update src/main/java/org/jabref/gui/groups/GroupDialog.fxml
m-peeler Jul 13, 2024
4931eac
Removed context menu option, disabled 'include selected' when editing…
m-peeler Jul 13, 2024
f928a36
Fixed imports
m-peeler Jul 13, 2024
93f2df5
Merge branch 'main' into right-click-create-group
Siedlerchr Jul 24, 2024
0367f7b
Merge branch 'main' into right-click-create-group
koppor Aug 4, 2024
e87af8e
Merge branch 'main' into right-click-create-group
m-peeler Sep 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ 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)

### Changed

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 7 additions & 1 deletion src/main/java/org/jabref/gui/StateManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,12 @@ public class StateManager {
private final ObservableList<String> searchHistory = FXCollections.observableArrayList();

public StateManager() {
activeGroups.bind(Bindings.valueAt(selectedGroups, activeDatabase.orElseOpt(null).map(BibDatabaseContext::getUid)));
koppor marked this conversation as resolved.
Show resolved Hide resolved
activeGroups.bind(Bindings.valueAt(
selectedGroups,
activeDatabase
.orElseOpt(null)
.map(BibDatabaseContext::getUid))
.orElse(FXCollections.observableArrayList()));
}

public ObservableList<SidePaneType> getVisibleSidePaneComponents() {
Expand Down Expand Up @@ -161,6 +166,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));
}
}
Expand Down
20 changes: 20 additions & 0 deletions src/main/java/org/jabref/gui/actions/ActionHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,26 @@ 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convert to JavaDoc

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());
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/org/jabref/gui/actions/StandardActions.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +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")),
ADD_GROUP_FROM_SELECTION(Localization.lang("Add group from selection")),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed? -- (I don't find the link to the comment I gave, maybe you remember the screenshot that "All entries" always have the context menu with "subgroups", not" groups")

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),
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/gui/exporter/ExportCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@

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 javafx.collections.FXCollections;
import javafx.stage.FileChooser;
import javafx.util.Duration;

Expand Down Expand Up @@ -111,7 +111,7 @@ private void export(Path file, FileChooser.ExtensionFilter selectedExtensionFilt
// All entries
entries = stateManager.getActiveDatabase()
.map(BibDatabaseContext::getEntries)
.orElse(Collections.emptyList());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change needed? I checked the caller - and it seems it is passed to org.jabref.logic.exporter.Exporter#export(org.jabref.model.database.BibDatabaseContext, java.nio.file.Path, java.util.List<org.jabref.model.entry.BibEntry>, java.util.List<java.nio.file.Path>, org.jabref.logic.journals.JournalAbbreviationRepository), which requires a list and nothing observable.

I would even try to use toList() since the list should not be modified (should it?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I just did this because the function signature changed (from Optional<List<..>> to Optional<ObservableList<...>> and I was trying to minimally fix to comply. Will add in the toList call and change it back.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will just revert and see if the CI is green.

Reason: BibDatbase context code:

    public List<BibEntry> getEntries() {
        return database.getEntries();
    }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did my revert go through, because I did not see it?

Please double check that this file is not changed!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per my earlier comment -

This push changes the method signature of getEntries() (since it was already an ObservableList and something needed to be able to bind to it). Removing the .map(List::stream).map(Stream::toList) causes compile errors and seems to be the reason tests are currently failing. I've changed it to instead do .orElse(FXCollections.emptyObservableList()) and that seems to address the compile issue.

.orElse(FXCollections.emptyObservableList());
}

List<Path> fileDirForDatabase = stateManager.getActiveDatabase()
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/org/jabref/gui/groups/GroupDialog.fxml
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,12 @@
<Tooltip text="%Group containing entries cited in a given TeX file"/>
</tooltip>
</RadioButton>
<RadioButton fx:id="selectionRadioButton" toggleGroup="$type" wrapText="true"
text="%Current selection">
<tooltip>
<Tooltip text="%Group all entries currently selected"/>
</tooltip>
</RadioButton>
</VBox>
<Separator orientation="VERTICAL"/>
<StackPane HBox.hgrow="ALWAYS">
Expand Down
36 changes: 29 additions & 7 deletions src/main/java/org/jabref/gui/groups/GroupDialogView.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -81,6 +82,7 @@ public class GroupDialogView extends BaseDialog<AbstractGroup> {
@FXML private RadioButton searchRadioButton;
@FXML private RadioButton autoRadioButton;
@FXML private RadioButton texRadioButton;
@FXML private RadioButton selectionRadioButton;

// Option Groups
@FXML private TextField keywordGroupSearchTerm;
Expand Down Expand Up @@ -109,6 +111,8 @@ public class GroupDialogView extends BaseDialog<AbstractGroup> {
private final BibDatabaseContext currentDatabase;
private final @Nullable GroupTreeNode parentNode;
private final @Nullable AbstractGroup editedGroup;
private final List<BibEntry> selectedEntries;
private final boolean preferUseSelection;

private GroupDialogViewModel viewModel;

Expand All @@ -119,10 +123,22 @@ public class GroupDialogView extends BaseDialog<AbstractGroup> {
public GroupDialogView(BibDatabaseContext currentDatabase,
@Nullable GroupTreeNode parentNode,
@Nullable AbstractGroup editedGroup,
GroupDialogHeader groupDialogHeader) {
GroupDialogHeader groupDialogHeader,
List<BibEntry> selectedEntries) {
this(currentDatabase, parentNode, editedGroup, groupDialogHeader, selectedEntries, false);
}

public GroupDialogView(BibDatabaseContext currentDatabase,
@Nullable GroupTreeNode parentNode,
@Nullable AbstractGroup editedGroup,
GroupDialogHeader groupDialogHeader,
List<BibEntry> selectedEntries,
boolean preferUseSelection) {
this.currentDatabase = currentDatabase;
this.parentNode = parentNode;
this.editedGroup = editedGroup;
this.selectedEntries = selectedEntries;
this.preferUseSelection = preferUseSelection;

ViewLoader.view(this)
.load()
Expand Down Expand Up @@ -172,7 +188,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, preferUseSelection);

setResultConverter(viewModel::resultConverter);

Expand Down Expand Up @@ -200,6 +216,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());
Expand Down Expand Up @@ -247,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();
});

Expand All @@ -261,10 +279,14 @@ public void initialize() {
viewModel.colorFieldProperty().setValue(IconTheme.getDefaultGroupColor());
return;
}
List<Color> colorsOfSiblings = parentNode.getChildren().stream().map(child -> child.getGroup().getColor())
.flatMap(Optional::stream)
.toList();
Optional<Color> parentColor = parentGroup().getColor();

List<Color> colorsOfSiblings = parentNode.getChildren()
.stream()
.map(GroupTreeNode::getGroup)
.map(AbstractGroup::getColor)
.flatMap(Optional::stream)
.toList();
Optional<Color> parentColor = parentNode.getGroup().getColor();
Color color;
color = parentColor.map(value -> GroupColorPicker.generateColor(colorsOfSiblings, value)).orElseGet(() -> GroupColorPicker.generateColor(colorsOfSiblings));
viewModel.colorFieldProperty().setValue(color);
Expand Down
53 changes: 49 additions & 4 deletions src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The heading does not match the following things.

Please move the Boolean Property.

private final StringProperty keywordGroupSearchTermProperty = new SimpleStringProperty("");
Expand All @@ -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;
Expand All @@ -112,24 +115,41 @@ public class GroupDialogViewModel {
private final AbstractGroup editedGroup;
private final GroupTreeNode parentNode;
private final FileUpdateMonitor fileUpdateMonitor;
private final List<BibEntry> selectedEntries;
private final boolean useSelectedEntries;

public GroupDialogViewModel(DialogService dialogService,
BibDatabaseContext currentDatabase,
PreferencesService preferencesService,
@Nullable AbstractGroup editedGroup,
@Nullable GroupTreeNode parentNode,
FileUpdateMonitor fileUpdateMonitor) {
FileUpdateMonitor fileUpdateMonitor,
List<BibEntry> 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();

Expand Down Expand Up @@ -246,7 +266,7 @@ private void setupValidation() {
return false;
}
return FileUtil.getFileExtension(input)
.map(extension -> "aux".equalsIgnoreCase(extension))
.map("aux"::equalsIgnoreCase)
.orElse(false);
}
},
Expand Down Expand Up @@ -371,6 +391,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) {
Expand Down Expand Up @@ -405,7 +433,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 {
Expand Down Expand Up @@ -535,7 +572,7 @@ public ValidationStatus keywordSearchTermEmptyValidationStatus() {
return keywordSearchTermEmptyValidator.getValidationStatus();
}

public ValidationStatus texGroupFilePathValidatonStatus() {
public ValidationStatus texGroupFilePathValidationStatus() {
return texGroupFilePathValidator.getValidationStatus();
}

Expand Down Expand Up @@ -587,6 +624,10 @@ public BooleanProperty typeTexProperty() {
return typeTexProperty;
}

public BooleanProperty typeSelectionProperty() {
return typeSelectionProperty;
}

public StringProperty keywordGroupSearchTermProperty() {
return keywordGroupSearchTermProperty;
}
Expand Down Expand Up @@ -638,4 +679,8 @@ public StringProperty autoGroupPersonsFieldProperty() {
public StringProperty texGroupFilePathProperty() {
return texGroupFilePathProperty;
}

public BooleanProperty entriesAreSelectedProperty() {
return entriesAreSelected;
}
}
Loading
Loading