-
Notifications
You must be signed in to change notification settings - Fork 126
fix: [ANDROAPP-7235] Text base inputs not saving when clicking on other inputs [skip size] #4539
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: develop
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR fixes an issue where text-based input fields were not saving their values when users clicked on other inputs, addressing JIRA issue ANDROAPP-7235. The solution introduces a focus change handler that triggers a save operation when text input fields lose focus.
Key Changes
- Added
onFieldFocusChangedhelper function to handle save-on-blur behavior for text inputs - Updated 14 text-based input components to include
onFocusChangedcallbacks - Ensures data persistence when switching between form fields
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| form/src/main/java/org/dhis2/form/ui/provider/OnFocusChangedAction.kt | New helper function that triggers FormIntent.OnSave when a field loses focus |
| form/src/main/java/org/dhis2/form/ui/provider/inputfield/InputsForTextValueTypeProvider.kt | Added onFocusChanged callbacks to QR code, default text, and barcode input fields |
| form/src/main/java/org/dhis2/form/ui/provider/inputfield/FieldProvider.kt | Added onFocusChanged callbacks to 11 numeric and text input types (integer, number, percentage, email, phone, link, long text, letter) |
| inline fun onFieldFocusChanged( | ||
| fieldUid: String, | ||
| value: String, | ||
| valueType: ValueType?, | ||
| isFocused: Boolean, | ||
| intentHandler: (FormIntent) -> Unit, | ||
| ) { | ||
| if (!isFocused) { | ||
| intentHandler( | ||
| FormIntent.OnSave( | ||
| fieldUid, | ||
| value, | ||
| valueType, | ||
| ), | ||
| ) |
Copilot
AI
Dec 11, 2025
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 logic in this helper function is duplicated 14 times across InputsForTextValueTypeProvider.kt and FieldProvider.kt. This creates maintainability concerns as any change to the save-on-blur behavior would need to be applied in 14 places.
Consider refactoring this to accept a lambda or use a higher-order function pattern to eliminate duplication. For example, you could create a modifier extension function or a composable wrapper that handles the focus change logic.
| inline fun onFieldFocusChanged( | |
| fieldUid: String, | |
| value: String, | |
| valueType: ValueType?, | |
| isFocused: Boolean, | |
| intentHandler: (FormIntent) -> Unit, | |
| ) { | |
| if (!isFocused) { | |
| intentHandler( | |
| FormIntent.OnSave( | |
| fieldUid, | |
| value, | |
| valueType, | |
| ), | |
| ) | |
| /** | |
| * Handles save-on-blur logic for a field. Call this function when the focus state changes. | |
| * | |
| * @param isFocused Whether the field is currently focused. | |
| * @param onSave Lambda to invoke when focus is lost (blur). | |
| */ | |
| inline fun onFieldFocusChanged( | |
| isFocused: Boolean, | |
| onSave: () -> Unit, | |
| ) { | |
| if (!isFocused) { | |
| onSave() |
| inline fun onFieldFocusChanged( | ||
| fieldUid: String, | ||
| value: String, | ||
| valueType: ValueType?, | ||
| isFocused: Boolean, | ||
| intentHandler: (FormIntent) -> Unit, | ||
| ) { | ||
| if (!isFocused) { | ||
| intentHandler( | ||
| FormIntent.OnSave( | ||
| fieldUid, | ||
| value, | ||
| valueType, | ||
| ), | ||
| ) | ||
| } | ||
| } |
Copilot
AI
Dec 11, 2025
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.
This new focus change behavior that triggers FormIntent.OnSave when focus is lost lacks test coverage. While there are comprehensive tests for FormIntent.OnSave handling in FormViewModelTest.kt and FormRepositoryImplTest.kt, there are no tests that verify the onFocusChanged callback is properly triggered and saves values when switching between text input fields.
Consider adding tests that verify:
- When focus changes from one text input to another, FormIntent.OnSave is called with the correct field UID and value
- No save is triggered when a field gains focus (only when it loses focus)
- The behavior works correctly for all affected input types
6bf0b11 to
fb8d498
Compare
0ef032e to
7158b2b
Compare
…it becomes idle randomly
Signed-off-by: Pablo Pajuelo Cabezas <pablo@dhis2.org>
c01ad2f to
0f05fff
Compare
|



Description
Link the JIRA issue.
Please provide a clear definition of the problem and explain your solution.