Remove react hook form in send email action #9130
Conversation
thomtrp
commented
Dec 18, 2024
thomtrp
commented
- remove react hook form
- put back multiline for body
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Simplified form management in WorkflowEditActionFormSendEmail by removing react-hook-form dependency in favor of native React state management.
- Replaced complex form controllers with useState hook in
/packages/twenty-front/src/modules/workflow/workflow-actions/components/WorkflowEditActionFormSendEmail.tsx - Added multiline support to email body field via FormTextFieldInput component
- Implemented handleFieldChange function for direct form state updates
- Streamlined form submission with debounced save action
1 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
...y-front/src/modules/workflow/workflow-actions/components/WorkflowEditActionFormSendEmail.tsx
Show resolved
Hide resolved
...y-front/src/modules/workflow/workflow-actions/components/WorkflowEditActionFormSendEmail.tsx
Show resolved
Hide resolved
| setFormData({ | ||
| connectedAccountId: action.settings.input.connectedAccountId, | ||
| email: action.settings.input.email, | ||
| subject: action.settings.input.subject ?? '', | ||
| body: action.settings.input.body ?? '', | ||
| }); | ||
| }, [action.settings.input]); |
There was a problem hiding this comment.
Honestly, I'm not sure if we need this useEffect. If the server takes time to respond, the form's state might be corrupted.
| useEffect(() => { | ||
| form.setValue( | ||
| 'connectedAccountId', | ||
| action.settings.input.connectedAccountId ?? '', | ||
| ); | ||
| form.setValue('email', action.settings.input.email ?? ''); | ||
| form.setValue('subject', action.settings.input.subject ?? ''); | ||
| form.setValue('body', action.settings.input.body ?? ''); | ||
| }, [action.settings, form]); | ||
| setFormData({ | ||
| connectedAccountId: action.settings.input.connectedAccountId, | ||
| email: action.settings.input.email, | ||
| subject: action.settings.input.subject ?? '', | ||
| body: action.settings.input.body ?? '', | ||
| }); | ||
| }, [action.settings.input]); |
There was a problem hiding this comment.
After reviewing this, I would remove the useEffect. We built the form field inputs with a defaultValue in mind, not a controlled value. Technically, our abstractions don't allow us to set the value of TipTap editors.
This useEffect has no effect, at least for form fields. The connected account select is a controlled component, and this useEffect would set its value. I think it's better to consider that the back-end state doesn't override the local state. We'll see how we can handle it properly if that's okay with you. Preventing the local override makes the experience on a slow network better. I prefer not to override what people might have typed in inputs to display the fresher data coming from the backend.
...y-front/src/modules/workflow/workflow-actions/components/WorkflowEditActionFormSendEmail.tsx
Outdated
Show resolved
Hide resolved
thomtrp
force-pushed
the
tt-fix-send-email-multiline
branch
from
9870ea2 to
12aacd1
Compare
