fixed workflow issues #3149
fixed workflow issues #3149
Conversation
📝 WalkthroughWalkthroughThis change set updates UI and navigation behavior in the personal ID feature. In the personal ID message screen layout, the text label is removed from a hidden button. The name input field now automatically capitalizes the first letter of each word. In the navigation graph, the Sequence Diagram(s)sequenceDiagram
participant User
participant App UI
participant PhoneHintLauncher
User->>App UI: Navigates to Personal ID Phone Fragment
App UI->>PhoneHintLauncher: Launch phone hint request
PhoneHintLauncher-->>App UI: Returns result (OK/data or not)
alt Result OK and data present
App UI->>App UI: Handle phone hint normally
else Result not OK or data null
App UI->>App UI: Focus phone input field
end
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
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: 0
🧹 Nitpick comments (1)
app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java (1)
110-112: Good implementation with minor formatting issue.The logic correctly handles the case when phone hint retrieval fails or is canceled by focusing the phone input field. Using
post()to schedule the focus request on the UI thread is the proper approach.Minor formatting issue: Add a space after
else:- }else{ + } else {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/res/layout/screen_personalid_message.xml(0 hunks)app/res/layout/screen_personalid_name.xml(1 hunks)app/res/navigation/nav_graph_personalid.xml(1 hunks)app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java(1 hunks)
💤 Files with no reviewable changes (1)
- app/res/layout/screen_personalid_message.xml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (2)
app/res/layout/screen_personalid_name.xml (1)
72-72: LGTM! Proper implementation of auto-capitalization for name input.The addition of
textCapWordsto the input type correctly implements the requirement to auto-capitalize each word in the name field. This is a standard Android approach and improves user experience by automatically formatting names properly.app/res/navigation/nav_graph_personalid.xml (1)
78-78:Details
✅ Verification successful
Good type safety improvement - verify existing navigation calls.
Making the
buttonTextargument explicitly non-nullable improves type safety and ensures that a valid button text is always provided to the message display dialog.Verify that all existing navigation calls to
personalid_message_displayprovide non-null values for thebuttonTextargument:
🏁 Script executed:
#!/bin/bash # Description: Find all navigation calls to personalid_message_display and verify buttonText usage # Expected: All calls should provide non-null buttonText values # Search for navigation calls to the message display dialog rg -A 10 -B 5 "personalid_message_display|PersonalidMessageDisplay" --type java # Also search for any navigation direction usages rg -A 5 "navigateToMessageDisplay|actionPersonalidPhoneFragmentToPersonalidMessageDisplay" --type javaLength of output: 6676
🏁 Script executed:
#!/bin/bash # Show the buttonText argument definition in the nav graph rg -n -A 3 -B 3 '<argument.*name="buttonText"' app/res/navigation/nav_graph_personalid.xmlLength of output: 92
🏁 Script executed:
#!/bin/bash # Locate the navigation graph file FILE=$(find . -type f -path '*navigation*/nav_graph_personalid.xml') echo "Navigation graph file: $FILE" # List all argument definitions in the file rg -n '<argument' "$FILE" # Show context around buttonText argument rg -n -C 3 'name="buttonText"' "$FILE"Length of output: 768
buttonTextnon-nullability is safeAll existing navigation calls to
personalid_message_displayexplicitly pass a non-null value (getString(R.string.ok)) for thebuttonTextargument:
- app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java
PersonalIdPhoneFragmentDirections.actionPersonalidPhoneFragmentToPersonalidMessageDisplay(..., getString(R.string.ok), …)- app/src/org/commcare/fragments/personalId/PersonalIdBiometricConfigFragment.java
PersonalIdBiometricConfigFragmentDirections.actionPersonalidBiometricConfigToPersonalidMessage(..., getString(R.string.ok), …)Enforcing
app:nullable="false"onbuttonTextwill not break existing calls.
…nto pm_workflow_fix
|
@damagatchi retest this please |
| private void navigateToSelf() { | ||
| Navigation.findNavController(binding.getRoot()) | ||
| .navigate(PersonalIdBackupCodeFragmentDirections | ||
| .actionPersonalidBackupcodeSelf()); | ||
| } | ||
| } |
There was a problem hiding this comment.
this opens the same fragment again ? If so why do we want to do this ?
There was a problem hiding this comment.
yes it opens it again with the refresh view now it will behave as a new setup of the backup code as we have changed the isRecovery to false
There was a problem hiding this comment.
why do we need to load the whole fragment again ? Can't we just refresh the UI state in the current fragment directly ?
There was a problem hiding this comment.
39d094f
i have change the flow, now ui will refresh earlier i did this because of get feel that we are changing the flow and if later there is need to display any message on clicking of isnt me button
app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java
Show resolved
Hide resolved
shubham1g5
left a comment
shubham1g5
left a comment
There was a problem hiding this comment.
Looks good to me except removal of broadcast listener, we should also avoid adding random things to PR once a review iteration has happened but PR those changes separately.
|
@shubham1g5 as per the stackoverflow i found the solution which is in this commit 8d9bbe8 and tested also working fine |
|
@damagatchi retest this please |
Product Description
Fixed work flow issue raised by Dave
End of workflow: Button shouldn't say “Back”
Name page: Each word should auto-capitalize in name input
Phone page: Keyboard dismissed when I cancel phone picker dialog
Removed brodcast reciever for sms autofill for this relase
Technical Summary
Feature Flag
Safety Assurance
Safety story
Automated test coverage
QA Plan
Labels and Review