fix checkbox visibility #92
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #92 +/- ##
==========================================
- Coverage 74.85% 74.70% -0.15%
==========================================
Files 38 38
Lines 501 502 +1
Branches 93 94 +1
==========================================
Hits 375 375
Misses 90 90
- Partials 36 37 +1 ☔ View full report in Codecov by Sentry. |
shiplet
force-pushed
the
b/_/fix-checkbox-visibility-handler
branch
from
637a4b0 to
a042f9e
Compare
| @@ -19,6 +19,8 @@ export function BooleanControl({ | |||
| config, | |||
| description, | |||
| }: ControlProps) { | |||
| if (!visible) return null | |||
There was a problem hiding this comment.
There is a subtle but important distinction between display:none and visibility:hidden CSS when it comes to forms. The former unmounts from the DOM completely whereas the latter keeps the element present and available to maintain form state. It looks like antd has a hidden prop on Form.Item to support the latter.
Does it make sense for this visibility prop to act like display:none?
There was a problem hiding this comment.
There was a problem hiding this comment.
I think in the case of a JSONForms Control we want the item fully removed from the DOM because if it's not visible, then the implication is its values shouldn't be evaluated or included in the form payload.
This also follows the existing pattern we've established with other visibility-affected Control components.
|
🎉 This PR is included in version 1.21.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
We're passing
visibilityas a prop into the BooleanControl component but not doing anything with it as relates to visibility of the rendered checkbox. This leads to unexpected behaviors when modifying visibility rules in downstream applications.This PR adds the same
props.visiblecheck that's seen in other control elements.