-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Refine tab to move focus even when the last item is a button #13938
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?
Conversation
…ves focus to the next tab's field
# Conflicts: # jabgui/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java # jabgui/src/main/java/org/jabref/gui/fieldeditors/EditorTextField.java
…ves focus to the next tab's field
…es focus to the next tab's field
Co-authored-by: Carl Christian Snethlage <[email protected]>
…to fix-issue-11937 # Conflicts: # jabgui/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java
|
||
@Override | ||
protected TextInputControl createTextInputControl() { | ||
protected TextInputControl createTextInputControl(Field field) { |
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.
Would like if you could clarify, how is this change helping?
I don't see the field parameter being used anywhere 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.
The parameter isn’t used in this override, but it’s required to match the parent method’s signature. I could add a suppress annotation 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.
If one is having to do that, it's probably a bad use case for overriding
Will think over this
|
||
@Override | ||
protected TextInputControl createTextInputControl() { | ||
protected TextInputControl createTextInputControl(@SuppressWarnings("unused") Field field) { |
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.
Can't _ be used?
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.
not using 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.
What was meant is, can't you replace the suppresswarnings by just renaming the parameter to _
?
It's a standard practice to mark them as placeholders to be ignored
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.
thanks for the info, but it is not allowing me to name like that (even though im using java 24)
jabgui/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java
Outdated
Show resolved
Hide resolved
ClipBoardManager.addX11Support(this); | ||
} | ||
|
||
public void setupTabNavigation(Predicate<TextField> isLastFieldChecker, Runnable nextTabSelector) { |
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 method 'setupTabNavigation' allows null values for 'isLastFieldChecker' and 'nextTabSelector', which could lead to runtime errors. Consider using Optional to avoid null checks.
@trag-bot didn't find any issues in the code! ✅✨ |
Your code currently does not meet JabRef's code guidelines. IntelliJ auto format covers some cases. There seem to be issues with your code style and autoformat configuration. Please reformat your code (Ctrl+Alt+L) and commit, then push. In special cases, consider using |
@tejjgv please do not resolve comments yourself (unless the change is very trivial). If the person who asked for the change (or someone else who is reviewing next) has verified that your changes are correct, they will resolve the conversation themselves. |
Sorry, I will not resolve comments myself from now on |
Closes #11937
Description
Improve tab behavior to move focus even when the last item is a button
Limit focus changes to fields within the EntryEditor, preventing jumps to unrelated UI components.
Introduce static helper methods in EditorTextField to handle TAB key behavior.
Ensure all TextField instances in SimpleEditor, CitationKeyEditor, PersonsEditor, and MarkdownEditor have proper IDs, enabling correct detection of the last field.
Steps to test
Mandatory checks
CHANGELOG.md
in a way that is understandable for the average user (if change is visible to the user)Screen.Recording.2025-09-13.155910.mp4