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

KeyCode on TextArea DoneEvent #3816

Merged
merged 9 commits into from
Jun 17, 2024
Merged

Conversation

ImmediandoSrl
Copy link
Contributor

PR relates to #3814

Added the keycode that triggered the done event to the ActionEvent that is sent.

@shai-almog
Copy link
Collaborator

You can't change the signature of the fireDoneEvent() method. That's a public API, you will be breaking compatibility with older versions. You can only add another method with the same name and the additional parameter. The old method can invoke the new method to prevent code duplication but you can't break the code of everyone who relied on fireDoneEvent().

Other than that looks good. If I would nitpick I would revert the change toActionEvent.java since it's just indentation. But that's not a big deal.

@shai-almog
Copy link
Collaborator

Also notice the compilation error from CI:

    [javac] Compiling 636 source files to /home/runner/work/CodenameOne/CodenameOne/CodenameOne/build/classes
    [javac] /home/runner/work/CodenameOne/CodenameOne/CodenameOne/src/com/codename1/ui/TextArea.java:2205: error: local variable keyEvent is accessed from within inner class; needs to be declared final
    [javac]                         fireDoneEvent(keyEvent);

@ImmediandoSrl
Copy link
Contributor Author

yes sorry for ActionEvent, i was editing the class from tablet yesterday and i didn't find the revert button for ActionEvent class.

Regarding the TextArea, I changed the fireDoneEvent because, looking in the code, the only call was in the InPlaceEditView, so changing them would not have created incompatibility problems. I still updated the class as you asked

@shai-almog shai-almog merged commit 1bf72d5 into codenameone:master Jun 17, 2024
1 check passed
@shai-almog
Copy link
Collaborator

Thanks.

Once a method is public in a framework anyone anywhere can rely on that. Changing a public method would mean breaking code you don't know about. For future PRs please try to include a JavaDoc comment for every method otherwise it looks weird in the JavaDoc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants