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

Added keybinding for renaming group with F2 #12159

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions src/main/java/org/jabref/gui/actions/StandardActions.java
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ public enum StandardActions implements Action {
GROUP_REMOVE_KEEP_SUBGROUPS(Localization.lang("Keep subgroups")),
GROUP_REMOVE_WITH_SUBGROUPS(Localization.lang("Also remove subgroups")),
GROUP_CHAT(Localization.lang("Chat with group")),
GROUP_RENAME(Localization.lang("Rename group"), KeyBinding.RENAME_GROUP),
GROUP_EDIT(Localization.lang("Edit group")),
GROUP_GENERATE_SUMMARIES(Localization.lang("Generate summaries for entries in the group")),
GROUP_GENERATE_EMBEDDINGS(Localization.lang("Generate embeddings for linked files in the group")),
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/groups/GroupDialogView.java
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ public class GroupDialogView extends BaseDialog<AbstractGroup> {
private final EnumMap<GroupHierarchyType, String> hierarchyToolTip = new EnumMap<>(GroupHierarchyType.class);

private final ControlsFxVisualizer validationVisualizer = new ControlsFxVisualizer();

private final BibDatabaseContext currentDatabase;
private final @Nullable GroupTreeNode parentNode;
private final @Nullable AbstractGroup editedGroup;
Expand All @@ -118,6 +117,7 @@ public GroupDialogView(BibDatabaseContext currentDatabase,
@Nullable GroupTreeNode parentNode,
@Nullable AbstractGroup editedGroup,
GroupDialogHeader groupDialogHeader) {

this.currentDatabase = currentDatabase;
this.parentNode = parentNode;
this.editedGroup = editedGroup;
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/org/jabref/gui/groups/GroupTreeView.java
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,7 @@ private ContextMenu createContextMenuForGroup(GroupNodeViewModel group) {
ActionFactory factory = new ActionFactory();

MenuItem removeGroup;

if (group.hasSubgroups() && group.canAddGroupsIn() && !group.isRoot()) {
removeGroup = new Menu(Localization.lang("Remove group"), null,
factory.createMenuItem(StandardActions.GROUP_REMOVE_KEEP_SUBGROUPS,
Expand All @@ -557,6 +558,7 @@ private ContextMenu createContextMenuForGroup(GroupNodeViewModel group) {
factory.createMenuItem(StandardActions.GROUP_EDIT, new ContextAction(StandardActions.GROUP_EDIT, group)),
factory.createMenuItem(StandardActions.GROUP_GENERATE_EMBEDDINGS, new ContextAction(StandardActions.GROUP_GENERATE_EMBEDDINGS, group)),
factory.createMenuItem(StandardActions.GROUP_GENERATE_SUMMARIES, new ContextAction(StandardActions.GROUP_GENERATE_SUMMARIES, group)),
factory.createMenuItem(StandardActions.GROUP_RENAME, new ContextAction(StandardActions.GROUP_RENAME, group)),
removeGroup,
new SeparatorMenuItem(),
factory.createMenuItem(StandardActions.GROUP_SUBGROUP_ADD, new ContextAction(StandardActions.GROUP_SUBGROUP_ADD, group)),
Expand Down Expand Up @@ -639,6 +641,8 @@ public ContextAction(StandardActions command, GroupNodeViewModel group) {
switch (command) {
case GROUP_EDIT ->
group.isEditable();
case GROUP_RENAME ->
group.isEditable();
case GROUP_REMOVE, GROUP_REMOVE_WITH_SUBGROUPS, GROUP_REMOVE_KEEP_SUBGROUPS ->
group.isEditable() && group.canRemove();
case GROUP_SUBGROUP_ADD ->
Expand All @@ -662,6 +666,10 @@ public void execute() {
switch (command) {
case GROUP_REMOVE ->
viewModel.removeGroupNoSubgroups(group);
case GROUP_RENAME -> {
viewModel.renameGroup(group);
groupTree.refresh();
}
case GROUP_REMOVE_KEEP_SUBGROUPS ->
viewModel.removeGroupKeepSubgroups(group);
case GROUP_REMOVE_WITH_SUBGROUPS ->
Expand Down
44 changes: 44 additions & 0 deletions src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,50 @@ boolean onlyMinorChanges(AbstractGroup oldGroup, AbstractGroup newGroup) {
return true;
}

/**
* Opens "Rename Group Dialog" and change name of group
*/
public void renameGroup(GroupNodeViewModel oldGroup) {
currentDatabase.ifPresent(database -> {
AbstractGroup oldGroupDef = oldGroup.getGroupNode().getGroup();
String oldGroupName = oldGroupDef.getName();

Optional<AbstractGroup> newGroup = dialogService.showCustomDialogAndWait(
new RenameGroupView(database,
oldGroup.getGroupNode().getGroup())
);

newGroup.ifPresent(group -> {

Copy link
Member

Choose a reason for hiding this comment

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

Pleaase - not that many empty lines. Especially not after {.

String newGroupName = group.getName();

if (newGroupName.trim().isEmpty()) {
return;
}

if (newGroupName.contains(",")) {
return;
}
Comment on lines +284 to +286
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't JabRef have a check for new group names?

What about the dialog to add new groups?

If the check is also in there, maybe, the method can be extracted?


if (oldGroupName.equals(newGroupName)) {
return;
}
Comment on lines +288 to +290
Copy link
Member

Choose a reason for hiding this comment

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

That should go first.


int groupsWithSameName = 0;
Optional<GroupTreeNode> databaseRootGroup = currentDatabase.get().getMetaData().getGroups();
if (databaseRootGroup.isPresent()) {
groupsWithSameName = databaseRootGroup.get().findChildrenSatisfying(g -> g.getName().equals(newGroupName)).size();
Copy link
Member

Choose a reason for hiding this comment

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

There needs to be comment why this is necessary. At first guess, I would have thought that there must not be any other group with the same name. Why is one other group with the same name OK?

}

if (groupsWithSameName < 2) {
oldGroup.getGroupNode().setGroup(group, true, true, database.getEntries());
writeGroupChangesToMetaData();
refresh();
}
});
});
}

/**
* Opens "Edit Group Dialog" and changes the given group to the edited one.
*/
Expand Down
78 changes: 78 additions & 0 deletions src/main/java/org/jabref/gui/groups/RenameGroupView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package org.jabref.gui.groups;

import java.util.Optional;

import javafx.fxml.FXML;
import javafx.scene.control.ButtonType;
import javafx.scene.control.Label;
import javafx.scene.control.TextField;
import javafx.scene.layout.VBox;

import org.jabref.gui.DialogService;
import org.jabref.gui.util.BaseDialog;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.groups.AbstractGroup;

import jakarta.inject.Inject;

public class RenameGroupView extends BaseDialog<AbstractGroup> {

@FXML
private TextField nameField;

@Inject
private DialogService dialogService;


private final BibDatabaseContext currentDatabase;
private final AbstractGroup editedGroup;

public RenameGroupView(BibDatabaseContext currentDatabase,
AbstractGroup editedGroup) {
this.currentDatabase = currentDatabase;
this.editedGroup = editedGroup;

setWidth(400);
setHeight(150);

setTitle(Localization.lang("Rename group"));

getDialogPane().getButtonTypes().setAll(ButtonType.OK, ButtonType.CANCEL);

nameField = new TextField();
nameField.setPromptText(Localization.lang("Enter new group name"));

VBox vbox = new VBox(new Label(Localization.lang("New group name")), nameField);
getDialogPane().setContent(vbox);

setResultConverter(buttonType -> {
if (buttonType == ButtonType.OK) {
return resultConverter(ButtonType.OK).orElse(null);
} else {
return null;
}
});
}

protected Optional<AbstractGroup> resultConverter(ButtonType button) {
if (button != ButtonType.OK) {
return Optional.empty();
}

try {
String newGroupName = nameField.getText().trim();

if (editedGroup != null) {
editedGroup.nameProperty().setValue(newGroupName);
return Optional.of(editedGroup);
}

return Optional.empty();
} catch (Exception exception) {
dialogService.showErrorDialogAndWait(exception.getLocalizedMessage(), exception);
return Optional.empty();
}
}
}

3 changes: 2 additions & 1 deletion src/main/java/org/jabref/gui/keyboard/KeyBinding.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@
* @implNote Cannot be sorted alphabetically, as {@link KeyBindingRepository#getKeyCombination(KeyBinding)} iterates over the enum in order and returns the first match.
*/
public enum KeyBinding {
EDITOR_DELETE("Delete", Localization.lang("Delete text"), "", KeyBindingCategory.EDITOR),
// DELETE BACKWARDS = Rubout
EDITOR_BACKWARD("Move caret left", Localization.lang("Move caret left"), "", KeyBindingCategory.EDITOR),
EDITOR_FORWARD("Move caret right", Localization.lang("Move caret right"), "", KeyBindingCategory.EDITOR),
EDITOR_DELETE("Delete", Localization.lang("Delete text"), "", KeyBindingCategory.EDITOR),
RENAME_GROUP("Rename Group", Localization.lang("Rename group"), "F2", KeyBindingCategory.EDIT),
EDITOR_WORD_BACKWARD("Move caret to previous word", Localization.lang("Move caret to previous word"), "", KeyBindingCategory.EDITOR),
EDITOR_WORD_FORWARD("Move caret to next word", Localization.lang("Move caret to next word"), "", KeyBindingCategory.EDITOR),
EDITOR_BEGINNING("Move caret to beginning of line", Localization.lang("Move caret to beginning of line"), "", KeyBindingCategory.EDITOR),
Expand Down
Loading