Skip to content
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

Don't render AddressNL component when it hidden #786

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

robinmolen
Copy link
Contributor

Partly closes: open-formulieren/open-forms#4699

The AddressNL component should only be rendered when it's not hidden. Otherwise, the attach function will call createRoot with an argument that isn't a DOM element. (Which causes an error being thrown in the front-end..)

So the creating of the root and, therefor, the rendering of the React component cannot be done. If the reactRoot doesn't exist, then it also doesn't (and cannot) be unmounted. So this is also something that shouldn't happen when the component is hidden.

This works fine when the component is hidden/shown using form logic, as the hidden property will be updated.

…when not hidden

The AddressNL component should only be rendered when it's not hidden. Otherwise, the attach function will call `createRoot` with an argument that isn't a DOM element.

So the creating of the root and, therefor, the rendering of the React component cannot be done.
If the reactRoot doesn't exist, then it also doesn't (and cannot) be unmounted. So this is also something that shouldn't happen when the component is hidden.

This works fine when the component is hidden/shown using form logic, as the `hidden` property will be updated.
Copy link

codecov bot commented Jan 27, 2025

Bundle Report

Changes will increase total bundle size by 292 bytes (0.0%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
@open-formulieren/sdk-OpenForms-umd 4.75MB 146 bytes (0.0%) ⬆️
@open-formulieren/sdk-esm 4.75MB 146 bytes (0.0%) ⬆️

Copy link

codecov bot commented Jan 27, 2025

Codecov Report

Attention: Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 83.31%. Comparing base (49bd7cb) to head (bf05e4a).
Report is 25 commits behind head on main.

Files with missing lines Patch % Lines
src/formio/components/AddressNL.jsx 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #786      +/-   ##
==========================================
- Coverage   83.32%   83.31%   -0.02%     
==========================================
  Files         239      239              
  Lines        4750     4753       +3     
  Branches     1268     1277       +9     
==========================================
+ Hits         3958     3960       +2     
- Misses        757      758       +1     
  Partials       35       35              
Flag Coverage Δ
storybook 75.77% <83.33%> (-0.01%) ⬇️
vitest 62.60% <0.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@vaszig vaszig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I a bit confused..which part of the ticket is this solving?

  • I see here we have a css rule for hidden components but I guess this is not useful in this situation. And where do we handle the "hidden" components and prevent them from being rendered?

https://github.com/open-formulieren/open-forms-sdk/blob/main/src/scss/components/_formio-component.scss#L39

https://github.com/open-formulieren/open-forms-sdk/blob/main/src/formio/components/Component.js

@vaszig
Copy link
Contributor

vaszig commented Jan 27, 2025

  • I a bit confused..which part of the ticket is this solving?

    • I see here we have a css rule for hidden components but I guess this is not useful in this situation. And where do we handle the "hidden" components and prevent them from being rendered?

https://github.com/open-formulieren/open-forms-sdk/blob/main/src/scss/components/_formio-component.scss#L39

https://github.com/open-formulieren/open-forms-sdk/blob/main/src/formio/components/Component.js

Discussed these remarks/questions in person with Robin, so for me this update in the code makes sense.

@sergei-maertens sergei-maertens merged commit 82df116 into main Jan 28, 2025
17 checks passed
@sergei-maertens sergei-maertens deleted the bug/4699-dont-render-hidden-components branch January 28, 2025 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AddressNL validation on city & postcode issues
3 participants