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.

Sign up for GitHub

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

Remove react hook form in send email action #9130

Merged
merged 2 commits into from
Dec 19, 2024
Merged

Remove react hook form in send email action #9130

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
355169a
Add multiline + remove react hook form
thomtrp Dec 18, 2024
12aacd1
Remove useEffects
thomtrp Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 75 additions & 97 deletions ...ront/src/modules/workflow/workflow-actions/components/WorkflowEditActionFormSendEmail.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@ import { useFindManyRecords } from '@/object-record/hooks/useFindManyRecords';
import { FormTextFieldInput } from '@/object-record/record-field/form-types/components/FormTextFieldInput';
import { useTriggerApisOAuth } from '@/settings/accounts/hooks/useTriggerApiOAuth';
import { Select, SelectOption } from '@/ui/input/components/Select';
import { WorkflowStepBody } from '@/workflow/components/WorkflowStepBody';
import { WorkflowStepHeader } from '@/workflow/components/WorkflowStepHeader';
import { WorkflowVariablePicker } from '@/workflow/components/WorkflowVariablePicker';
import { workflowIdState } from '@/workflow/states/workflowIdState';
import { WorkflowSendEmailAction } from '@/workflow/types/Workflow';
import { useTheme } from '@emotion/react';
import { useEffect } from 'react';
import { Controller, useForm } from 'react-hook-form';
import { useEffect, useState } from 'react';
import { useRecoilValue } from 'recoil';
import { IconMail, IconPlus, isDefined } from 'twenty-ui';
import { JsonValue } from 'type-fest';
import { useDebouncedCallback } from 'use-debounce';
import { WorkflowStepBody } from '@/workflow/components/WorkflowStepBody';

type WorkflowEditActionFormSendEmailProps = {
action: WorkflowSendEmailAction;
Expand Down Expand Up @@ -47,14 +47,11 @@ export const WorkflowEditActionFormSendEmail = ({
const workflowId = useRecoilValue(workflowIdState);
const redirectUrl = `/object/workflow/${workflowId}`;

const form = useForm<SendEmailFormData>({
defaultValues: {
connectedAccountId: '',
email: '',
subject: '',
body: '',
},
disabled: actionOptions.readonly,
const [formData, setFormData] = useState<SendEmailFormData>({
connectedAccountId: action.settings.input.connectedAccountId,
email: action.settings.input.email,
subject: action.settings.input.subject ?? '',
body: action.settings.input.body ?? '',
});
thomtrp marked this conversation as resolved.
Show resolved Hide resolved

const checkConnectedAccountScopes = async (
Expand All @@ -79,14 +76,13 @@ export const WorkflowEditActionFormSendEmail = ({
};

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]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Honestly, I'm not sure if we need this useEffect. If the server takes time to respond, the form's state might be corrupted.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

thomtrp marked this conversation as resolved.
Show resolved Hide resolved

const saveAction = useDebouncedCallback(
async (formData: SendEmailFormData, checkScopes = false) => {
Expand Down Expand Up @@ -120,10 +116,19 @@ export const WorkflowEditActionFormSendEmail = ({
};
}, [saveAction]);

const handleSave = (checkScopes = false) =>
form.handleSubmit((formData: SendEmailFormData) =>
saveAction(formData, checkScopes),
)();
const handleFieldChange = (
fieldName: keyof SendEmailFormData,
updatedValue: JsonValue,
) => {
thomtrp marked this conversation as resolved.
Show resolved Hide resolved
const newFormData: SendEmailFormData = {
...formData,
[fieldName]: updatedValue,
};

setFormData(newFormData);

saveAction(newFormData);
};

const filter: { or: object[] } = {
or: [
Expand Down Expand Up @@ -190,83 +195,56 @@ export const WorkflowEditActionFormSendEmail = ({
headerType="Email"
/>
<WorkflowStepBody>
<Controller
name="connectedAccountId"
control={form.control}
render={({ field }) => (
<Select
dropdownId="select-connected-account-id"
label="Account"
fullWidth
emptyOption={emptyOption}
value={field.value}
options={connectedAccountOptions}
callToActionButton={{
onClick: () =>
triggerApisOAuth('google', {
redirectLocation: redirectUrl,
}),
Icon: IconPlus,
text: 'Add account',
}}
onChange={(connectedAccountId) => {
field.onChange(connectedAccountId);
handleSave(true);
}}
disabled={actionOptions.readonly}
/>
)}
<Select
dropdownId="select-connected-account-id"
label="Account"
fullWidth
emptyOption={emptyOption}
value={formData.connectedAccountId}
options={connectedAccountOptions}
callToActionButton={{
onClick: () =>
triggerApisOAuth('google', {
redirectLocation: redirectUrl,
}),
Icon: IconPlus,
text: 'Add account',
}}
onChange={(connectedAccountId) => {
handleFieldChange('connectedAccountId', connectedAccountId);
}}
disabled={actionOptions.readonly}
/>
<Controller
name="email"
control={form.control}
render={({ field }) => (
<FormTextFieldInput
label="Email"
placeholder="Enter receiver email"
readonly={actionOptions.readonly}
defaultValue={field.value}
onPersist={(value) => {
field.onChange(value);
handleSave();
}}
VariablePicker={WorkflowVariablePicker}
/>
)}
<FormTextFieldInput
label="Email"
placeholder="Enter receiver email"
readonly={actionOptions.readonly}
defaultValue={formData.email}
onPersist={(email) => {
handleFieldChange('email', email);
}}
VariablePicker={WorkflowVariablePicker}
/>
<Controller
name="subject"
control={form.control}
render={({ field }) => (
<FormTextFieldInput
label="Subject"
placeholder="Enter email subject"
readonly={actionOptions.readonly}
defaultValue={field.value}
onPersist={(value) => {
field.onChange(value);
handleSave();
}}
VariablePicker={WorkflowVariablePicker}
/>
)}
<FormTextFieldInput
label="Subject"
placeholder="Enter email subject"
readonly={actionOptions.readonly}
defaultValue={formData.subject}
onPersist={(subject) => {
handleFieldChange('subject', subject);
}}
VariablePicker={WorkflowVariablePicker}
/>
<Controller
name="body"
control={form.control}
render={({ field }) => (
<FormTextFieldInput
label="Body"
placeholder="Enter email body"
readonly={actionOptions.readonly}
defaultValue={field.value}
onPersist={(value) => {
field.onChange(value);
handleSave();
}}
VariablePicker={WorkflowVariablePicker}
/>
)}
<FormTextFieldInput
label="Body"
placeholder="Enter email body"
readonly={actionOptions.readonly}
defaultValue={formData.body}
onPersist={(body) => {
handleFieldChange('body', body);
}}
VariablePicker={WorkflowVariablePicker}
multiline
/>
</WorkflowStepBody>
</>
Expand Down
Loading