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 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
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
3 changes: 3 additions & 0 deletions src/main/java/org/jabref/gui/groups/GroupDialogView.java
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ public GroupDialogView(BibDatabaseContext currentDatabase,
@Nullable GroupTreeNode parentNode,
@Nullable AbstractGroup editedGroup,
GroupDialogHeader groupDialogHeader) {

this.currentDatabase = currentDatabase;
this.parentNode = parentNode;
this.editedGroup = editedGroup;
Expand Down Expand Up @@ -158,6 +159,8 @@ public GroupDialogView(BibDatabaseContext currentDatabase,
confirmDialogButton.disableProperty().bind(viewModel.validationStatus().validProperty().not());
// handle validation before closing dialog and calling resultConverter
confirmDialogButton.addEventFilter(ActionEvent.ACTION, viewModel::validationHandler);


}

private @Nullable AbstractGroup parentGroup() {
Expand Down
12 changes: 12 additions & 0 deletions src/main/java/org/jabref/gui/groups/GroupTreeView.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Collectors;
import javafx.scene.control.ContextMenu;
import javafx.scene.control.MenuItem;
import javafx.scene.control.Button;
import javafx.scene.layout.BorderPane;
import javafx.scene.layout.HBox;

import javafx.application.Platform;
import javafx.beans.binding.Bindings;
Expand Down Expand Up @@ -538,6 +543,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 +563,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 +646,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 +671,9 @@ public void execute() {
switch (command) {
case GROUP_REMOVE ->
viewModel.removeGroupNoSubgroups(group);
case GROUP_RENAME -> {
viewModel.renameGroup(group);
}
case GROUP_REMOVE_KEEP_SUBGROUPS ->
viewModel.removeGroupKeepSubgroups(group);
case GROUP_REMOVE_WITH_SUBGROUPS ->
Expand Down
59 changes: 59 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,9 +260,68 @@ boolean onlyMinorChanges(AbstractGroup oldGroup, AbstractGroup newGroup) {
return true;
}

/**
* Opens "Rename Group Dialog" and change name of group
*/

Copy link
Member

Choose a reason for hiding this comment

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

Why the empt yline?

Copy link
Author

Choose a reason for hiding this comment

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

Removed empty line

public void renameGroup(GroupNodeViewModel oldGroup) {
currentDatabase.ifPresent(database -> {
AbstractGroup oldGroupDef = oldGroup.getGroupNode().getGroup();
String oldGroupName = oldGroupDef.getName(); // Zachytenie starého názvu pred otvorením dialógu
Copy link
Member

Choose a reason for hiding this comment

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

Please translate the comment (or remove it)

Copy link
Author

Choose a reason for hiding this comment

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

removed



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

newGroup.ifPresent(group -> {

String newGroupName = group.getName();


//check if it´s empty new string
if (newGroupName.trim().isEmpty()) {
dialogService.notify(Localization.lang("The new group name cannot be empty or consist only of spaces."));
return;
}

// check if is not include ","
Copy link
Member

Choose a reason for hiding this comment

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

Remove comment - the next line says the same.

The WHY is important. Why is this check done.

Copy link
Author

Choose a reason for hiding this comment

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

Hello. I removed this comment. I check this because I inspired by Edit group where this check too. I can remove this if-statement if you want.

if (newGroupName.contains(",")) {
dialogService.notify(Localization.lang("The new group name cannot contain commas."));
return;
}

// chceck if old group name dont equels with new group name
Copy link
Member

Choose a reason for hiding this comment

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

Remove comment - next line says it

Copy link
Author

Choose a reason for hiding this comment

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

removed

if (oldGroupName.equals(newGroupName)) {
dialogService.notify(Localization.lang("The new group name is the same as the old one. No changes made."));
return;
}


// check if other groups name are not same
int groupsWithSameName = 0;
Optional<GroupTreeNode> databaseRootGroup = currentDatabase.get().getMetaData().getGroups();
if (databaseRootGroup.isPresent()) {
groupsWithSameName = databaseRootGroup.get().findChildrenSatisfying(g -> g.getName().equals(newGroupName)).size();
}

// then change name
Rydzard marked this conversation as resolved.
Show resolved Hide resolved
if (groupsWithSameName < 2) {
oldGroup.getGroupNode().setGroup(group, true, true, database.getEntries());
dialogService.notify(Localization.lang("Renamed group from \"%0\" to \"%1\".", oldGroupName, newGroupName));
writeGroupChangesToMetaData();
refresh();
}
});
});
}


/**
* Opens "Edit Group Dialog" and changes the given group to the edited one.
*/

public void editGroup(GroupNodeViewModel oldGroup) {
currentDatabase.ifPresent(database -> {
Optional<AbstractGroup> newGroup = dialogService.showCustomDialogAndWait(new GroupDialogView(
Expand Down
83 changes: 83 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,83 @@
package org.jabref.gui.groups;

import jakarta.inject.Inject;
import javafx.fxml.FXML;
import javafx.scene.control.*;
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.*;
import org.jspecify.annotations.Nullable;

import java.util.Optional;

public class RenameGroupView extends BaseDialog<AbstractGroup> {

@FXML
private TextField nameField;

@Inject
private DialogService dialogService;


private final BibDatabaseContext currentDatabase;
private final @Nullable AbstractGroup editedGroup;

public RenameGroupView(BibDatabaseContext currentDatabase,
@Nullable AbstractGroup editedGroup) {
Copy link
Member

Choose a reason for hiding this comment

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

What happens if this is null? - I think, this also also should not be null? - Please add JavaDoc if it can be null

Copy link
Author

Choose a reason for hiding this comment

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

I removed Nulls. And it still works fine.

this.currentDatabase = currentDatabase;
this.editedGroup = editedGroup;

// set Width and Height
Copy link
Member

Choose a reason for hiding this comment

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

remove

Copy link
Author

Choose a reason for hiding this comment

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

I removed

setWidth(400);
setHeight(150);

// set Title name
Copy link
Member

Choose a reason for hiding this comment

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

REmove

Copy link
Author

Choose a reason for hiding this comment

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

I removed

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

// add OK and Cancel buttons
Copy link
Member

Choose a reason for hiding this comment

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

Remove

Copy link
Author

Choose a reason for hiding this comment

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

I removed

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

nameField = new TextField();
nameField.setPromptText("Enter new group name");
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

added


// add Input
Copy link
Member

Choose a reason for hiding this comment

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

Remove

Copy link
Author

Choose a reason for hiding this comment

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

I removed

VBox vbox = new VBox(new Label("New group name:"), nameField);
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

done

getDialogPane().setContent(vbox);

// If press OK change name else return null
Copy link
Member

Choose a reason for hiding this comment

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

renive

Copy link
Author

Choose a reason for hiding this comment

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

I removed

setResultConverter(buttonType -> {
if (buttonType == ButtonType.OK) {
return resultConverter(ButtonType.OK).orElse(null);
} else {
// Ak sa zvolí Cancel alebo sa dialóg zavrie cez X
Copy link
Member

Choose a reason for hiding this comment

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

translate

Copy link
Author

Choose a reason for hiding this comment

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

I removed

return null;
}
});
}

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

try {
// Get new name from Input
String newGroupName = nameField.getText().trim();

if (editedGroup != null) {
//set new input
editedGroup.nameProperty().set(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,7 +6,8 @@
* @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),
EDITOR_DELETE("Delete", Localization.lang("Delete text"), "",KeyBindingCategory.EDITOR ),
RENAME_GROUP("Rename Group",Localization.lang("Rename group"),"R", KeyBindingCategory.EDIT),
// 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),
Expand Down
Loading