-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat(dashboard): add delay step #7131
Changes from 18 commits
146e9e2
4eae38e
b1c6f9f
5c1378e
07642c1
c2b3c52
2a08599
bdbf903
4b6ea51
688568b
0e45fc2
54946fc
4c989cb
ef822ac
4c59b6f
2bbe057
b76624f
44f83c5
a751111
2e89ea1
c5adc57
77548aa
143e5ba
68ad9c1
b59f853
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,104 @@ | ||||||
import { cn } from '@/utils/ui'; | ||||||
import { FormControl, FormField, FormItem, FormMessagePure } from '@/components/primitives/form/form'; | ||||||
import { Input } from '@/components/primitives/input'; | ||||||
import { InputFieldPure } from '@/components/primitives/input'; | ||||||
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/primitives/select'; | ||||||
import { useFormContext } from 'react-hook-form'; | ||||||
import { useMemo } from 'react'; | ||||||
|
||||||
type InputWithSelectProps = { | ||||||
fields: { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nested in props is always 👿 . how about using two flat props?
This should simplify the logic of the handleChange a lot. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @SokratisVidros this is what we agreed before with @desiprisg and we use this pattern in other places where the component is using two form fields There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't a flat structure much easier to work with? |
||||||
inputKey: string; | ||||||
selectKey: string; | ||||||
}; | ||||||
options: string[]; | ||||||
defaultOption?: string; | ||||||
ChmaraX marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
className?: string; | ||||||
placeholder?: string; | ||||||
isReadOnly?: boolean; | ||||||
}; | ||||||
|
||||||
export const NumberInputWithSelect = ({ | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its a generic component not necessarily a duration, it can be also Lets see how it evolves with the digest and we will incrementally adjust this component, by either making it primitive or whatever we need - I dont want to make it too generic too early since I dont know what will digest need. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The it's an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer That said I changed it to |
||||||
fields, | ||||||
options, | ||||||
defaultOption, | ||||||
className, | ||||||
placeholder, | ||||||
isReadOnly, | ||||||
}: InputWithSelectProps) => { | ||||||
const { getFieldState, setValue, getValues, control } = useFormContext(); | ||||||
ChmaraX marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
const input = getFieldState(`${fields.inputKey}`); | ||||||
const select = getFieldState(`${fields.selectKey}`); | ||||||
const error = input.error || select.error; | ||||||
|
||||||
const defaultSelectedValue = useMemo(() => { | ||||||
return defaultOption ?? options[0]; | ||||||
}, [defaultOption, options]); | ||||||
|
||||||
const handleChange = (value: { input: number; select: string }) => { | ||||||
// we want to always set both values and treat it as a single input | ||||||
setValue(fields.inputKey, value.input, { shouldDirty: true }); | ||||||
setValue(fields.selectKey, value.select, { shouldDirty: true }); | ||||||
}; | ||||||
|
||||||
return ( | ||||||
<> | ||||||
<InputFieldPure className="h-7 rounded-lg border pr-0"> | ||||||
ChmaraX marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
<FormField | ||||||
control={control} | ||||||
name={fields.inputKey} | ||||||
render={({ field }) => ( | ||||||
<FormItem className="w-full overflow-hidden"> | ||||||
<FormControl> | ||||||
<Input | ||||||
type="number" | ||||||
ChmaraX marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
className={cn( | ||||||
'min-w-[20ch] [appearance:textfield] [&::-webkit-inner-spin-button]:appearance-none [&::-webkit-outer-spin-button]:appearance-none', | ||||||
className | ||||||
)} | ||||||
placeholder={placeholder} | ||||||
disabled={isReadOnly} | ||||||
{...field} | ||||||
onChange={(e) => { | ||||||
handleChange({ input: Number(e.target.value), select: getValues(fields.selectKey) }); | ||||||
}} | ||||||
/> | ||||||
</FormControl> | ||||||
</FormItem> | ||||||
)} | ||||||
/> | ||||||
<FormField | ||||||
control={control} | ||||||
name={fields.selectKey} | ||||||
render={({ field }) => ( | ||||||
<FormItem> | ||||||
<FormControl> | ||||||
<Select | ||||||
onValueChange={(value) => { | ||||||
handleChange({ input: Number(getValues(fields.inputKey)), select: value }); | ||||||
}} | ||||||
defaultValue={defaultSelectedValue} | ||||||
disabled={isReadOnly} | ||||||
{...field} | ||||||
> | ||||||
<SelectTrigger className="h-7 w-auto translate-x-0.5 gap-1 rounded-l-none border-l bg-neutral-50 p-2 text-xs"> | ||||||
<SelectValue /> | ||||||
</SelectTrigger> | ||||||
<SelectContent> | ||||||
{options.map((option) => ( | ||||||
<SelectItem key={option} value={option}> | ||||||
{option} | ||||||
</SelectItem> | ||||||
))} | ||||||
</SelectContent> | ||||||
</Select> | ||||||
</FormControl> | ||||||
</FormItem> | ||||||
)} | ||||||
/> | ||||||
</InputFieldPure> | ||||||
<FormMessagePure error={error ? String(error.message) : undefined} /> | ||||||
ChmaraX marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
</> | ||||||
); | ||||||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
import { useCallback, useEffect, useMemo } from 'react'; | ||
import merge from 'lodash.merge'; | ||
import { useForm } from 'react-hook-form'; | ||
import { zodResolver } from '@hookform/resolvers/zod'; | ||
import type { StepDataDto, StepIssuesDto, UpdateWorkflowDto, WorkflowResponseDto } from '@novu/shared'; | ||
import { StepTypeEnum } from '@novu/shared'; | ||
|
||
import { flattenIssues, updateStepControlValuesInWorkflow } from '@/components/workflow-editor/step-utils'; | ||
import { buildDefaultValues, buildDefaultValuesOfDataSchema, buildDynamicZodSchema } from '@/utils/schema'; | ||
import { Form } from '@/components/primitives/form/form'; | ||
import { useFormAutosave } from '@/hooks/use-form-autosave'; | ||
import { SaveFormContext } from '@/components/workflow-editor/steps/save-form-context'; | ||
import { DelayForm } from '@/components/workflow-editor/steps/delay/delay-form'; | ||
|
||
const STEP_TYPE_TO_INLINE_FORM: Record<StepTypeEnum, (args: StepInlineFormProps) => React.JSX.Element | null> = { | ||
ChmaraX marked this conversation as resolved.
Show resolved
Hide resolved
|
||
[StepTypeEnum.DELAY]: DelayForm, | ||
[StepTypeEnum.IN_APP]: () => null, | ||
[StepTypeEnum.EMAIL]: () => null, | ||
[StepTypeEnum.SMS]: () => null, | ||
[StepTypeEnum.CHAT]: () => null, | ||
[StepTypeEnum.PUSH]: () => null, | ||
[StepTypeEnum.CUSTOM]: () => null, | ||
[StepTypeEnum.TRIGGER]: () => null, | ||
[StepTypeEnum.DIGEST]: () => null, | ||
}; | ||
|
||
// Use the UI Schema to build the default values if it exists else use the data schema (code-first approach) values | ||
const calculateDefaultValues = (step: StepDataDto) => { | ||
if (Object.keys(step.controls.uiSchema ?? {}).length !== 0) { | ||
return merge(buildDefaultValues(step.controls.uiSchema ?? {}), step.controls.values); | ||
} | ||
|
||
return merge(buildDefaultValuesOfDataSchema(step.controls.dataSchema ?? {}), step.controls.values); | ||
}; | ||
|
||
export type StepInlineFormProps = { | ||
workflow: WorkflowResponseDto; | ||
step: StepDataDto; | ||
}; | ||
|
||
type ConfigureStepInlineFormProps = StepInlineFormProps & { | ||
issues?: StepIssuesDto; | ||
update: (data: UpdateWorkflowDto) => void; | ||
updateStepCache: (step: Partial<StepDataDto>) => void; | ||
}; | ||
|
||
export const ConfigureStepInlineForm = (props: ConfigureStepInlineFormProps) => { | ||
const { workflow, step, issues, update, updateStepCache } = props; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we are drilling the props quite deep, we should think about using the hooks here, but I left it like this for consistency. |
||
const schema = useMemo(() => buildDynamicZodSchema(step.controls.dataSchema ?? {}), [step.controls.dataSchema]); | ||
|
||
const defaultValues = useMemo(() => { | ||
return calculateDefaultValues(step); | ||
}, [step]); | ||
|
||
const form = useForm({ | ||
resolver: zodResolver(schema), | ||
defaultValues, | ||
shouldFocusError: false, | ||
}); | ||
|
||
const { onBlur, saveForm } = useFormAutosave({ | ||
previousData: defaultValues, | ||
form, | ||
save: (data) => { | ||
update(updateStepControlValuesInWorkflow(workflow, step, data)); | ||
updateStepCache({ ...step, controls: { ...step.controls, values: data } }); | ||
}, | ||
}); | ||
|
||
const setIssuesFromStep = useCallback(() => { | ||
const stepIssues = flattenIssues(issues?.controls); | ||
Object.entries(stepIssues).forEach(([key, value]) => { | ||
form.setError(key as string, { message: value }); | ||
}); | ||
}, [form, issues]); | ||
|
||
useEffect(() => { | ||
setIssuesFromStep(); | ||
}, [setIssuesFromStep]); | ||
|
||
const InlineForm = STEP_TYPE_TO_INLINE_FORM[step.type]; | ||
|
||
const value = useMemo(() => ({ saveForm }), [saveForm]); | ||
|
||
return ( | ||
<Form {...form}> | ||
<form className="flex h-full flex-col" onBlur={onBlur}> | ||
<SaveFormContext.Provider value={value}> | ||
<InlineForm workflow={workflow} step={step} /> | ||
</SaveFormContext.Provider> | ||
</form> | ||
</Form> | ||
); | ||
}; |
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 probably can't have it like this, see this thread for more info.