-
Notifications
You must be signed in to change notification settings - Fork 14
Chore: Replace formBuilder.tsx with Component Model PT1 of Many #6168
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: main
Are you sure you want to change the base?
Conversation
🧪 Review environmenthttps://mlw5doevmenm4zalfyvd2jzlqy0bdpzm.lambda-url.ca-central-1.on.aws/ |
| ? element.properties.validation.required | ||
| : false; | ||
|
|
||
| const descriptionPerLocale = element.properties[getLocalizedProperty("description", lang)]; |
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.
Maybe we should have helpers for pulling descriptions titles etc... ?
Just thinking about how repetitive this is going to get.
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.
Same with the Label, I've just been thinking through how I want to do it in case we need to break a label out of an element in other ways.
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.
I went with just making a linked component, but it feels a bit ugly. I'll keep thinking, and if anyone has a better idea.
| meta: { | ||
| type: FormElementTypes.textField, | ||
| }, | ||
| render({ element, lang }) { |
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.
Should we add a toString or renderString value ... for things like the review page / response downloads?
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.
Yes, did we want to make that change for this PR for textField as a proof of concept, or save it for the next one? I was going to break out validation as well.
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.
We can save it for the next one :)
…y Formik to instead.
…hange as expected.
A refactor of the formBuilder.tsx to adopt a component model with the final intent being components (eg: TextField, Checkbox, AddressComplete, FileInput, etc...) are fully self contained, and logic is not spread across the application.
This PR only implements the TextField component so that it can be tested heavily, and then a new PR can be span up for other components in parallel.