Handling Next and Done keys in Backup Code Fragment #3192
Conversation
…ent (numeric keypad).
📝 WalkthroughWalkthroughA new private method, Sequence Diagram(s)sequenceDiagram
participant User
participant BackupCodeInput
participant BackupCodeRepeatInput
participant PersonalIdBackupCodeFragment
User->>BackupCodeInput: Enter backup code + press "Next"/"Done"
BackupCodeInput->>PersonalIdBackupCodeFragment: onEditorAction
alt isRecovery is true
PersonalIdBackupCodeFragment->>PersonalIdBackupCodeFragment: submitBackupCode()
else isRecovery is false
PersonalIdBackupCodeFragment->>BackupCodeRepeatInput: requestFocus()
end
User->>BackupCodeRepeatInput: Enter repeat code + press "Next"/"Done"
BackupCodeRepeatInput->>PersonalIdBackupCodeFragment: onEditorAction
alt Continue button enabled
PersonalIdBackupCodeFragment->>PersonalIdBackupCodeFragment: submitBackupCode()
end
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java (1)
117-125: Duplicate listener logic could be factored to reduce maintenance overhead.
Both lambdas share the sameactionIdchecks and only differ in the branch that decides whether to submit the backup code. Extracting a small private helper (e.g.,boolean handleImeAction(int actionId)) or using a single listener instance would DRY up the code and keep future changes (e.g., addingIME_NULL) in one place.This is optional, but it will prevent subtle divergence between the two listeners over time.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java(3 hunks)
🔇 Additional comments (1)
app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java (1)
58-60: Sequence placement looks good.
CallingsetupDoneKeys()immediately aftersetupListeners()keeps all listener registration in one place and before any field manipulation (clearBackupCodeFields()), so there is no functional concern here.
| private void setupDoneKeys() { | ||
| binding.connectBackupCodeInput.setOnEditorActionListener((v, actionId, event) -> { | ||
| if (actionId == EditorInfo.IME_ACTION_NEXT || actionId == EditorInfo.IME_ACTION_DONE) { | ||
| if(isRecovery) { | ||
| handleBackupCodeSubmission(); | ||
| } else { | ||
| binding.connectBackupCodeRepeatInput.requestFocus(); | ||
| } | ||
| return true; | ||
| } | ||
| return false; | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
IME_ACTION_* alone may not catch hardware Enter/Return presses.
OnEditorActionListener is invoked with actionId == EditorInfo.IME_NULL when the user presses the hardware/soft-keyboard Enter key that is not mapped to an explicit IME action. Right now that path falls through and the key does nothing. Consider also checking actionId == EditorInfo.IME_NULL (and optionally event != null && event.getKeyCode() == KeyEvent.KEYCODE_ENTER) so that all Enter/Done presses are handled consistently.
- if (actionId == EditorInfo.IME_ACTION_NEXT || actionId == EditorInfo.IME_ACTION_DONE) {
+ if (actionId == EditorInfo.IME_ACTION_NEXT ||
+ actionId == EditorInfo.IME_ACTION_DONE ||
+ actionId == EditorInfo.IME_NULL) {Would you confirm whether the hardware Enter key currently submits the code on target devices?
Handle hardware Enter/Return presses in OnEditorActionListener
On some devices the hardware/soft-keyboard Enter key isn’t mapped to IME_ACTION_NEXT or IME_ACTION_DONE, so actionId becomes IME_NULL and your handler is skipped. Update the condition to also catch Enter presses:
File: app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java (around lines 104–115)
- if (actionId == EditorInfo.IME_ACTION_NEXT || actionId == EditorInfo.IME_ACTION_DONE) {
+ if (actionId == EditorInfo.IME_ACTION_NEXT
+ || actionId == EditorInfo.IME_ACTION_DONE
+ || (actionId == EditorInfo.IME_NULL
+ && event != null
+ && event.getKeyCode() == KeyEvent.KEYCODE_ENTER)) {This ensures that both IME actions and hardware Enter/Return submit the backup code as expected.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private void setupDoneKeys() { | |
| binding.connectBackupCodeInput.setOnEditorActionListener((v, actionId, event) -> { | |
| if (actionId == EditorInfo.IME_ACTION_NEXT || actionId == EditorInfo.IME_ACTION_DONE) { | |
| if(isRecovery) { | |
| handleBackupCodeSubmission(); | |
| } else { | |
| binding.connectBackupCodeRepeatInput.requestFocus(); | |
| } | |
| return true; | |
| } | |
| return false; | |
| }); | |
| private void setupDoneKeys() { | |
| binding.connectBackupCodeInput.setOnEditorActionListener((v, actionId, event) -> { | |
| - if (actionId == EditorInfo.IME_ACTION_NEXT || actionId == EditorInfo.IME_ACTION_DONE) { | |
| + if (actionId == EditorInfo.IME_ACTION_NEXT | |
| + || actionId == EditorInfo.IME_ACTION_DONE | |
| + || (actionId == EditorInfo.IME_NULL | |
| + && event != null | |
| + && event.getKeyCode() == KeyEvent.KEYCODE_ENTER)) { | |
| if (isRecovery) { | |
| handleBackupCodeSubmission(); | |
| } else { | |
| binding.connectBackupCodeRepeatInput.requestFocus(); | |
| } | |
| return true; | |
| } | |
| return false; | |
| }); | |
| } |
🤖 Prompt for AI Agents
In app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java
around lines 104 to 115, the OnEditorActionListener currently only handles
IME_ACTION_NEXT and IME_ACTION_DONE, missing hardware Enter/Return key presses
which report actionId as IME_NULL. Update the condition to also check if
actionId equals EditorInfo.IME_NULL and if the event is not null and its keyCode
is KeyEvent.KEYCODE_ENTER, so that pressing the hardware Enter key triggers the
same submission logic as the IME actions.
Does this not work for you already ? It's working for me without these changes. |
| if(isRecovery) { | ||
| handleBackupCodeSubmission(); | ||
| } else { | ||
| binding.connectBackupCodeRepeatInput.requestFocus(); |
There was a problem hiding this comment.
think this is not required.
There was a problem hiding this comment.
Changed it to rely on the default handler for that case: 8ec0edd
There was a problem hiding this comment.
The issue I had before was that the handler was returning true on any Next|Done event, even if it wasn't handled. Now returning false in that case, so the default handler moves the focus to the next input
| binding.connectBackupCodeInput.setOnEditorActionListener((v, actionId, event) -> { | ||
| if (actionId == EditorInfo.IME_ACTION_NEXT || actionId == EditorInfo.IME_ACTION_DONE) { | ||
| if(isRecovery) { | ||
| handleBackupCodeSubmission(); |
There was a problem hiding this comment.
should we also check if(binding.connectBackupCodeButton.isEnabled()) {
There was a problem hiding this comment.
It should also close the keyboard in both cases?
There was a problem hiding this comment.
Good catch: 8ec0edd
I decided to leave the keypad open since a wait dialog is displayed while the call is in progress (so the user can't press keys), and then the system automatically hides the keyboard when showing the failure message or proceeding to the next screen.
Relaying on default focus behavior when Next button pressed (from first to second backup code input).
https://dimagi.atlassian.net/browse/QA-7850
Added key handlers for the Next and Done keys when showing the numeric keypad in the backup code fragment.
When pressed:
-If in recovery: submits the code
-If new registration, first input: proceeds to second input
-If new registration, second input: proceeds to next page