-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
42616e2
ea811f1
8144433
20c387a
67a5d5f
5dcc5a2
487754a
a3e8599
bfa8228
645693b
75f84bc
068253c
d905fde
8bb211e
27f5690
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -260,9 +260,60 @@ 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(); // Zachytenie starého názvu pred otvorením dialógu | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please translate the comment (or remove it) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 -> { | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
} | ||
|
||
// check if is not include "," | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(",")) { | ||
return; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? |
||
|
||
// chceck if old group name dont equels with new group name | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove comment - next line says it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed |
||
if (oldGroupName.equals(newGroupName)) { | ||
return; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? |
||
} | ||
|
||
// then change name | ||
Rydzard marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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. | ||
*/ | ||
|
||
@SuppressWarnings("checkstyle:EmptyLineSeparator") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No - just remove these two lines and everything will be fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed |
||
public void editGroup(GroupNodeViewModel oldGroup) { | ||
currentDatabase.ifPresent(database -> { | ||
Optional<AbstractGroup> newGroup = dialogService.showCustomDialogAndWait(new GroupDialogView( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
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; | ||
import org.jspecify.annotations.Nullable; | ||
|
||
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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed |
||
setWidth(400); | ||
setHeight(150); | ||
|
||
// set Title name | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. REmove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed |
||
setTitle(Localization.lang("Rename group")); | ||
|
||
// add OK and Cancel buttons | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove There was a problem hiding this comment. Choose a reason for hiding this commentThe 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"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use Localiaztion.lang - see https://devdocs.jabref.org/code-howtos/localization.html There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added |
||
|
||
// add Input | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed |
||
VBox vbox = new VBox(new Label("New group name:"), nameField); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use Localiaztion.lang - see https://devdocs.jabref.org/code-howtos/localization.html No ":" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
getDialogPane().setContent(vbox); | ||
|
||
// If press OK change name else return null | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. renive There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. translate There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
editedGroup.nameProperty().setValue(newGroupName); | ||
return Optional.of(editedGroup); | ||
} | ||
|
||
return Optional.empty(); | ||
} catch (Exception exception) { | ||
dialogService.showErrorDialogAndWait(exception.getLocalizedMessage(), exception); | ||
return Optional.empty(); | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the empt yline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed empty line