Added preset for new entry keybindings and reintroduced defaults #7705
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| package org.jabref.gui.preferences.keybindings.presets; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
|
|
||
| import org.jabref.gui.keyboard.KeyBinding; | ||
|
|
||
| public class NewEntryBindingPreset implements KeyBindingPreset { | ||
|
|
||
| @Override | ||
| public String getName() { | ||
| return "New Entries"; | ||
|
There was a problem hiding this comment. I find "new entries" a bit confusing, maybe something longer that better explains the purpose? Should be localized as well. There was a problem hiding this comment. Please make a suggestion. We did not come up with something better yet. There was a problem hiding this comment. Something like "Bindings for creating all entry types"? There was a problem hiding this comment. Remember this is just shown in the small menubutton below the the keybindings table There was a problem hiding this comment. A long text would just repeat information the user already has and would be imho extremely annoying. There was a problem hiding this comment. Yeah, I couldn't find a nice formulation that doesn't include "Bindings". But with "New entries" or "New entry types" it's really not clear what this is doing. |
||
| } | ||
|
|
||
| @Override | ||
| public Map<KeyBinding, String> getKeyBindings() { | ||
| final Map<KeyBinding, String> keyBindings = new HashMap<>(); | ||
|
|
||
| // Clear conflicting default presets | ||
| keyBindings.put(KeyBinding.PULL_CHANGES_FROM_SHARED_DATABASE, ""); | ||
| keyBindings.put(KeyBinding.COPY_PREVIEW, ""); | ||
|
|
||
| // Add new entry presets | ||
| keyBindings.put(KeyBinding.NEW_ARTICLE, "Ctrl+shift+A"); | ||
| keyBindings.put(KeyBinding.NEW_BOOK, "Ctrl+shift+B"); | ||
| keyBindings.put(KeyBinding.NEW_ENTRY, "Ctrl+N"); | ||
| keyBindings.put(KeyBinding.NEW_ENTRY_FROM_PLAIN_TEXT, "Ctrl+shift+N"); | ||
| keyBindings.put(KeyBinding.NEW_INBOOK, "Ctrl+shift+I"); | ||
| keyBindings.put(KeyBinding.NEW_INPROCEEDINGS, "Ctrl+shift+C"); | ||
| keyBindings.put(KeyBinding.NEW_MASTERSTHESIS, "Ctrl+shift+M"); | ||
| keyBindings.put(KeyBinding.NEW_PHDTHESIS, "Ctrl+shift+T"); | ||
| keyBindings.put(KeyBinding.NEW_PROCEEDINGS, "Ctrl+shift+P"); | ||
| keyBindings.put(KeyBinding.NEW_TECHREPORT, "Ctrl+shift+R"); | ||
| keyBindings.put(KeyBinding.NEW_UNPUBLISHED, "Ctrl+shift+U"); | ||
|
|
||
| return keyBindings; | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Are these defaults really necessary? They were removed in a recent PR on purpose (because they are not often used). Users can still set keybindings themselves, right?
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.
Carl and me discussed this in length yesterday, too.
I like convention over configuration. If I start a tool, I want to have it behaving "well". It should require configuration only for certain special cases.
In the concrete case, the key bindings are NOT used for some other cases (except the pull from shared database). Thus, I have a strong vote to add the non-conflicting key bindings back as default.
The other option is that I write a lengthy manual on key bindings and explain each (power) user why they have to configure key bindings. - IMHO JabRef should not require much documentation. Only for the hard cases.
See also #7439 and #7346. Thus, there are multiple users used to the key bindings.
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.
There are too many entry types, especially if you use biblatex, to have keybindings for all types. Thus you need to make a cutoff somewhere, and in my opinion it should be sufficient to have bindings for the 5 most commonly used types. This would also be consistent with the "new entry" dialog where the following types are treated specially:
jabref/src/main/java/org/jabref/model/entry/types/BiblatexEntryTypeDefinitions.java
Line 446 in d9964c6
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.
The "most commonly used types" are for me the ones where keybindings existed for more than a decade. IMHO there is no need to remove functionality in this case.
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.
From this perspective, we shouldn't have changed the new entry dialog as well...