-
-
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
Fix issue #11938: Tab in empty text area field should focus the next field #12081
base: main
Are you sure you want to change the base?
Conversation
Add event filter to move focus on TAB key press when TextArea is 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.
Too much obvious code comments.
I did not see any screenshots. Thus, reviewers need to run the tool. I think I can do it these days.
@@ -26,7 +28,20 @@ public class EditorTextArea extends TextArea implements Initializable, ContextMe | |||
}; | |||
|
|||
public EditorTextArea() { | |||
// Call the constructor with an empty string. |
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.
Remove comment - the code says it.
You can add a WHY to the comment. Why the empty string is passed. (If you know)
this(""); | ||
|
||
// Add an event filter to handle key press events |
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.
Remove comment, the code says it.
|
||
// Add an event filter to handle key press events | ||
this.addEventFilter(KeyEvent.KEY_PRESSED, event -> { | ||
// If the TAB key is pressed and the TextArea is 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.
Remove comment, the code says it.
this.addEventFilter(KeyEvent.KEY_PRESSED, event -> { | ||
// If the TAB key is pressed and the TextArea is empty | ||
if (event.getCode() == KeyCode.TAB && this.getText().isEmpty()) { | ||
// Move the focus to the next parent element (likely the next form 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.
Remove comment, the code says it.
// Move the focus to the next parent element (likely the next form field) | ||
this.getParent().requestFocus(); | ||
|
||
// Consume the event to prevent further processing of the TAB key |
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.
Remove comment, the code says it.
# remove comment
Hi @koppor, Thank you for the feedback! I've now removed the obvious code comments as suggested. |
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.
You also need to add a CHANGELOG.md entry - as soon as you really fixed the issue. |
# remove comment
@@ -27,6 +29,12 @@ public class EditorTextArea extends TextArea implements Initializable, ContextMe | |||
|
|||
public EditorTextArea() { | |||
this(""); | |||
this.addEventFilter(KeyEvent.KEY_PRESSED, event -> { | |||
if (event.getCode() == KeyCode.TAB && this.getText() != null && this.getText().isEmpty()) { |
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.
You can use JabRefs StringUtil.isEmpty(this.getText())
This PR addresses the issue where pressing the Tab key in an empty text area should focus the next field, but it wasn't behaving as expected. The code change ensures that when the TextArea is empty, pressing the Tab key moves the focus to the next form field.
Closes #11938
Mandatory checks
CHANGELOG.md
described in a way that is understandable for the average user (if applicable)