-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Feat/field jumping 12276 #14120
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
base: main
Are you sure you want to change the base?
Feat/field jumping 12276 #14120
Conversation
… and autocorrect
Hey @ZachCraw!Thank you for contributing to JabRef! Your help is truly appreciated ❤️. We have automatic checks in place, based on which you will soon get automated feedback if any of them are failing. We also use TragBot with custom rules that scans your changes and provides some preliminary comments, before a maintainer takes a look. TragBot is still learning, and may not always be accurate. In the "Files changed" tab, you can go through its comments and just click on "Resolve conversation" if you are sure that it is incorrect, or comment on the conversation if you are doubtful. Please re-check our contribution guide in case of any other doubts related to our contribution workflow. |
|
|
||
| stateManager.getSelectedEntries().addListener((InvalidationListener) _ -> { | ||
| if (stateManager.getSelectedEntries().isEmpty()) { | ||
| // [impl->req~entry-editor.keep-showing~1] |
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.
please keep this as it referes to implementation spec
| ); | ||
| if (!stateManager.getSelectedEntries().isEmpty()) { | ||
| setCurrentlyEditedEntry(stateManager.getSelectedEntries().getFirst()); | ||
| Platform.runLater(() -> tabbed.requestFocus()); |
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 do you need Platform.runLater here?
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.
I found that this was required to solve a focus-handling issue. Without it, a "race condition" occurs when switching entries: the main table gains focus from the click, and the EntryEditor's simultaneous requestFocus() call is lost. This prevents the EntryEditor from receiving the Ctrl+J key event. Using Platform.runLater defers the focus request until after the table's selection event is processed, ensuring the EntryEditor reliably regains keyboard focus.
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.
So now you are always stealing the focus from the maintable and force it to the entry editor every time the user selects a different entry in the main table? I dont think that this is a desired behaviour.
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.
Yes. If this is not the desired behaviour, what would be? The ability to interact back with the other entries and main window while having the search box open?
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.
No, leaving the focus in the maintable. Think of a user navigating with the arrowkeys in the maintable.
| record JumpToField() implements UiCommand { | ||
| } | ||
|
|
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.
Is this used anywhere?
| case JUMP_TO_FIELD: | ||
| showJumpToFieldDialog(); | ||
| event.consume(); | ||
| break; |
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.
Maybe this should go in the scene hierarchy more up, if you want it to be callable from the entry editor and from the maintable alike.
Closes #12276
Co-developed with @evealitaylor
This pull request introduces a "Jump to Field" feature within the entry editor, accessible via the
Ctrl+Jshortcut. It enhances navigation by allowing users to quickly search for and jump to any field across all tabs. The implementation addresses previous issues with autocomplete by ensuring all possible fields are suggested, and it correctly manages focus to highlight the selected field and allow immediate editing.Steps to test
Ctrl+Jto open the "Jump to Field" dialog.Enter(or click "OK").Ctrl+Jagain. The "Jump to Field" dialog should successfully reappear.It should look like so:
2025-10-20.22-02-58.mp4
Mandatory checks
CHANGELOG.mdin a way that is understandable for the average user (if change is visible to the user)Current Issue