O3-4257: Fix obsGroup children visibility when parent hideWhenExpression evaluates to true#653
Conversation
…ion evaluates to true Problem: When an obsGroup had a hideWhenExpression that evaluated to true, the obsGroup and its child fields remained visible in the DOM. This was because: 1. The hide property was in the behaviours array and not extracted to field level 2. The ObsGroup component didn't check for hidden state 3. Child fields weren't marked as hidden when parent was hidden Solution: 1. Modified useEvaluateFormFieldExpressions to set isParentHidden flag on children when parent field is hidden 2. Updated ObsGroup component to: - Get the latest evaluated field state from formFields - Check if obsGroup itself is hidden (isHidden or isParentHidden) - Return null early if obsGroup should be hidden - Filter children to exclude hidden ones - Removed useMemo to allow proper re-rendering on state changes 3. Added comprehensive test case with form intent to ensure behaviours are processed Testing: - Created obs-group-hide-test-form.json with obsGroup having hideWhenExpression - Added test verifying obsGroup and children are removed from DOM when expression is true - Test verifies they reappear when expression becomes false - All existing tests continue to pass
|
@NethmiRodrigo sorry for tagging you again i have tried to fix this also can you have a look at it |
|
@denniskigen can you review this also please |
|
@NethmiRodrigo @denniskigen please have a look at this. |
Does the issue happen if the hide property is defined at the field level? |
|
@samuelmale You're right -in the current test form, the hide property is in the behaviours array and gets extracted to field level through applyFormIntent() in forms-loader.ts. I haven't tested the scenario where hide is defined directly at the field level (bypassing the behaviors extraction). Should I create a test case with the hide property defined directly at field level to verify if the issue occurs in both scenarios? |
|
Yes! To help narrow down the issue, we need to rule out whether it's related to how form behaviours are processed or with the obs-group component itself. Create a form with hide logic defined directly at the field level, test out this form on the upstream branch (without your changes). |
- Created obs-group-hide-direct-test-form.json with hide properties defined directly on fields instead of in behaviors array - Added corresponding test case to verify if obsgroup hide issue occurs with direct field-level hide properties - This helps narrow down whether the issue is with form behavior processing or obs-group component itself
|
@samuelmale i have done that please have a look |
|
@samuelmale please review it and please let me know if any further changes are required or is it good to merge ? |
Do the visibility issues happen when we define the hide logic at the field level? Can you share your findings? |
|
No, visibility issues do NOT occur when we define the hide logic at the field level. I've tested and verified both scenarios: Behavior-based hide logic (using behaviours array) Direct field-level hide logic (defined directly on the field) Both work correctly. Here's why: Both test cases pass successfully, confirming this behavior. |
|
@samuelmale sir any updates about it |
sir update please ! very eager to know whether it got fixed or not! 😊 |
This means that the issue is linked to how form intents (behaviours) are processed; that being said, here is the related logic. |
Hey Samuel, You were right - the issue was in the behavior processing logic. I added a check so child questions only process behaviors if they have them (matching parent logic), and made children inherit parent's |
| const { t } = useTranslation(); | ||
| const { formFieldAdapters, formFields } = useFormProviderContext(); | ||
|
|
||
| const content = useMemo( |
There was a problem hiding this comment.
Assuming the issue is related to how form intents are handled in the src/utils/forms-loader.ts, is it necessary to make changes in this file?
There was a problem hiding this comment.
Assuming the issue is related to how form intents are handled in the
src/utils/forms-loader.ts, is it necessary to make changes in this file?
I tested the fix by reverting obs-group.component.tsx to its original state (before commit eb663d8), and the tests FAILED without those changes. This proves that both fixes are necessary:
The forms-loader.ts fix ensures children inherit the parent's hide property at form load time.
The obs-group.component.tsx changes provide runtime filtering by checking isHidden and isParentHidden to properly hide the children.
The two layers work together one at form loading, one at rendering.
The testing demonstrated that:
Without obs-group.component.tsx changes: Tests FAIL (groups and children still render when they should be hidden)
|
@samuelmale an updates on this pr as i have implemented few changes! |
|
@samuelmale any update on how to move forward!? |
Requirements
Summary
Problem
When an
obsGroupfield has ahideWhenExpressionthat evaluates totrue, the obsGroup and its child fields remained visible in the DOM instead of being hidden. This breaks the expected conditional visibility behavior.Root Causes:
hideproperty was in thebehavioursarray and not extracted to field levelObsGroupcomponent didn't check for hidden stateuseMemowhich prevented re-rendering on state changesSolution
1. Hook Changes (
useEvaluateFormFieldExpressions.ts):isParentHiddenflag on child questions when parent field is hidden2. Component Changes (
obs-group.component.tsx):formFieldsto get current hidden statusisHiddenorisParentHidden)nullearly if obsGroup should be hidden (removes entire component from DOM)isHiddenorisParentHiddenflagsuseMemowrapper to allow proper re-rendering when field state changes3. Test Coverage:
obs-group-hide-test-form.jsonwith obsGroup havinghideWhenExpression'*') is passed to trigger proper behaviour extractionRelated Issue
https://issues.openmrs.org/browse/O3-5015