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

11802: add new KeyBinding for bibliographic data shortcut #11842

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/actions/StandardActions.java
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public enum StandardActions implements Action {
ABBREVIATE_DOTLESS(Localization.lang("dotless"), Localization.lang("Abbreviate journal names of the selected entries (DOTLESS abbreviation)")),
ABBREVIATE_SHORTEST_UNIQUE(Localization.lang("shortest unique"), Localization.lang("Abbreviate journal names of the selected entries (SHORTEST UNIQUE abbreviation)")),
UNABBREVIATE(Localization.lang("Unabbreviate journal names"), Localization.lang("Unabbreviate journal names of the selected entries"), KeyBinding.UNABBREVIATE),

GET_BIBLIOGRAPHIC_DATA(Localization.lang("Get bibliographic data"), IconTheme.JabRefIcons.SEARCH,KeyBinding.GET_BIBLIOGRAPHIC_DATA),
Copy link
Member

Choose a reason for hiding this comment

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

Please fix checkstyle: space messing

MANAGE_CUSTOM_EXPORTS(Localization.lang("Manage custom exports")),
MANAGE_CUSTOM_IMPORTS(Localization.lang("Manage custom imports")),
CUSTOMIZE_ENTRY_TYPES(Localization.lang("Customize entry types")),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
package org.jabref.gui.fieldeditors.identifier;

import java.util.Optional;

import javax.swing.undo.UndoManager;

import com.airhacks.afterburner.injection.Injector;
import com.airhacks.afterburner.views.ViewLoader;
import jakarta.inject.Inject;
import javafx.fxml.FXML;
import javafx.scene.Parent;
import javafx.scene.control.Button;
import javafx.scene.control.Tooltip;
import javafx.scene.input.KeyEvent;
import javafx.scene.layout.HBox;

import org.jabref.gui.DialogService;
import org.jabref.gui.StateManager;
import org.jabref.gui.autocompleter.SuggestionProvider;
Expand All @@ -18,33 +17,40 @@
import org.jabref.gui.fieldeditors.FieldEditorFX;
import org.jabref.gui.fieldeditors.contextmenu.DefaultMenu;
import org.jabref.gui.fieldeditors.contextmenu.EditorMenus;
import org.jabref.gui.keyboard.KeyBinding;
import org.jabref.gui.preferences.GuiPreferences;
import org.jabref.logic.integrity.FieldCheckers;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.util.TaskExecutor;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.Field;

import com.airhacks.afterburner.injection.Injector;
import com.airhacks.afterburner.views.ViewLoader;
import jakarta.inject.Inject;
import javax.swing.undo.UndoManager;
import java.util.Optional;

import static org.jabref.model.entry.field.StandardField.DOI;
import static org.jabref.model.entry.field.StandardField.EPRINT;
import static org.jabref.model.entry.field.StandardField.ISBN;
import static org.jabref.model.entry.field.StandardField.*;

public class IdentifierEditor extends HBox implements FieldEditorFX {

@FXML private BaseIdentifierEditorViewModel<?> viewModel;
@FXML private EditorTextArea textArea;
@FXML private Button fetchInformationByIdentifierButton;
@FXML private Button lookupIdentifierButton;

@Inject private DialogService dialogService;
@Inject private TaskExecutor taskExecutor;
@Inject private GuiPreferences preferences;
@Inject private UndoManager undoManager;
@Inject private StateManager stateManager;
@FXML
private BaseIdentifierEditorViewModel<?> viewModel;
@FXML
private EditorTextArea textArea;
@FXML
private Button fetchInformationByIdentifierButton;
@FXML
private Button lookupIdentifierButton;

@Inject
private DialogService dialogService;
@Inject
private TaskExecutor taskExecutor;
@Inject
private GuiPreferences preferences;
@Inject
private UndoManager undoManager;
@Inject
private StateManager stateManager;
Comment on lines +35 to +53
Copy link
Member

Choose a reason for hiding this comment

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

Please do not reformat the code. All JabRef code should be consistently formatted. Check our guidelines for setting up a workspace. With that configuration, this should not happen.


private Optional<BibEntry> entry = Optional.empty();

Expand All @@ -56,6 +62,9 @@ public IdentifierEditor(Field field,
// but we need the injected vars to create the viewmodels.
Injector.registerExistingAndInject(this);

initKeyBindings();


Comment on lines +66 to +67
Copy link
Member

Choose a reason for hiding this comment

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

No double empty lines.

switch (field) {
case DOI ->
this.viewModel = new DoiIdentifierEditorViewModel(suggestionProvider, fieldCheckers, dialogService, taskExecutor, preferences, undoManager, stateManager);
Expand All @@ -71,8 +80,8 @@ public IdentifierEditor(Field field,
}

ViewLoader.view(this)
.root(this)
.load();
.root(this)
.load();
Comment on lines -74 to +84
Copy link
Member

Choose a reason for hiding this comment

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

Please no code reformatting


textArea.textProperty().bindBidirectional(viewModel.textProperty());

Expand All @@ -90,6 +99,15 @@ public IdentifierEditor(Field field,
new EditorValidator(preferences).configureValidation(viewModel.getFieldValidator().getValidationStatus(), textArea);
}

private void initKeyBindings() {
addEventFilter(KeyEvent.KEY_PRESSED, event -> {
Optional<KeyBinding> keyBinding = preferences.getKeyBindingRepository().mapToKeyBinding(event);
if (keyBinding.isPresent() && keyBinding.get() == KeyBinding.GET_BIBLIOGRAPHIC_DATA) {
this.fetchInformationByIdentifier();
}
});
Comment on lines +103 to +108
Copy link
Member

Choose a reason for hiding this comment

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

This feels wrong. All other keybindings work differently. You already correctly added it into KeyBinding. - And you say, you need this peace of code here to get if fully working?

Copy link
Author

Choose a reason for hiding this comment

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

@koppor Just because I couldnt get it to work with org.jabref.gui.actions.ActionFactory#configureIconButton (locally).
And debugging it, didnt help either, i tried to understand it by using org.jabref.gui.groups.GroupDialogView#GroupDialogView (it configures a HelpAction) as a reference.
But also this doesnt seem to work for me locally.

When i have following modal open:
grafik

and press F1, nothing happens.

Then i stumpled upon org.jabref.gui.frame.JabRefFrame#initKeyBindings
which was the only occurence where I could observe a keybinding to be actually caught.

And then I copied that code and at least i now i have the wanted behavior.

}

public BaseIdentifierEditorViewModel<?> getViewModel() {
return viewModel;
}
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/jabref/gui/keyboard/KeyBinding.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
public enum KeyBinding {
EDITOR_DELETE("Delete", Localization.lang("Delete text"), "", KeyBindingCategory.EDITOR),
// DELETE BACKWARDS = Rubout
GET_BIBLIOGRAPHIC_DATA("Get bibliographic data", Localization.lang("Get bibliographic data"), "ctrl+shift+alt+B", KeyBindingCategory.EDITOR),
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_WORD_BACKWARD("Move caret to previous word", Localization.lang("Move caret to previous word"), "", KeyBindingCategory.EDITOR),
Expand Down
Loading