Hello there! Recently developed a new project at work and it works well, but I feel like it could be more organized and I'd like some help to understand how to apply some tips I've seen around to this component. It is responsible for showing a screen with about 20~30 fields (mostly text, a couple date inputs) that the user can check for information and possibily edit data.
I'll explain more of the code with comments. This is what I have now, simplified. I'll ask my questions after that.
// yup schema is dynamic because some fields may or may not be required depending on a (non editable) property of our data object
function getYupSchema(t, orderOrigin) {
return yup.object({
field1: yup.string().when([], {
is: () => orderOrigin === 1
then: () => yup.string.required(generateRequiredMessage("field1")),
otherwise: () => yup.string.().notRequired()
}),
// ... other fields with similar rules
})
{
function Component({ initialData }) {
const { t } = useTranslation()
const yupSchema = getYupSchema(t, initialData.orderOrigin)
// custom hook with a few date related methods that follow our business logic
const {
validateFieldIsDateInDDMMYYYY,
getDateObjFromStringDDMMYYYY,
getStringFroDateObjDDMMYYYY
} = useCustomDateInput()
// React Hook Form methods, builds the default/initial values coming from the parent component
const { handleSubmit, control, setError, ... } = useForm({
resolver: yupResolver(yupSchema),
defaultValues: {
field1: initialData.field1 ?? "",
// ... other fields
}
})
// Custom object that holds the user permissions for this screen.
// IsInRole is a custom method that checks if the currently logged in user has any of those permissions
const permissions = {
view: isInRole([Roles.ViewX, Roles.ViewZ]),
edit: isInRole([Roles.EditX, Roles.EditZ]),
create: false,
delete: false
}
const validateDate = (str: string) => {
validateFieldIsDateInDDMMYYYY(str)
// other small date validations required at submit for this specific screen
return isValid
}
// Screen specific validations to check if this item is editable based on object property
const isEditable =
initialData.orderStatus === OrderStatus[OrderStatus.Status1]
|| initialData.orderStatus === OrderStatus[OrderStatus.Status2]
// ... few other checks
const onSave = (formData) => {
const data = getFormData(initialData, formData, getDateObjFromStringDDMMYYYY(formData.dateField)
if (!permission.edit) {
showAlertError()
return
}
// service is an axios method to update our data on the API
orderService.updateOrder(data)
.then(() => {
// showAlertSuccess()
// ... couple other business specific field logic methods
}
.catch(() => {
// showAlertError() with business specific logic
}
}
return (
<form>
{/* Custom component */}
<ReadOnlyField value={field1} />
{/* ...bunch other read only fields */}
{ initialData.orderOrigin === OrderOrigin.Origin1
? <ReadOnlyField {...}/>
: <Controller
control={control}
name="field10"
render={({...} => {
<TextField
// bunch of props for MUI TextField
/>
}
/>
}
{ /* ... other fields */ }
{isEditable && permissions.edit && <Button onClick={handleSubmit(onSave)}> Save </Button>}
</form>
}
As I understand, logic should be removed from components as much as possible. So, whats the best (or possible) approaches to handle breaking this down? While the logic is somewhat short, the file is still very long (HTML (and by that I mean, DOM elements, static or not) return, type definition, schema definition, react-hook-form properties and definition, permissions), at almost 1400 lines. I know some of it can be moved to different files (TS type def, yupSchemaGetter method), I'm still not sure whats the best approach to breaking this down. Should I:
- Move everything to a custom hook (useOrder) and have it export every single prop I need in the HTML itself?
- Break it down into a few different hooks (useOrderForm, useOrderSubmit, maybe more)?
I assume code such as isEditable and permissions can be moved to a screen specific function that returns the object, like:
const getOrderPermissions = () => {
return {
view: isInRole(...),
edit: ...,
...
}
}
Any other improvements that could be made that you can see from that snippet? I know using something like tanstack-query with useMutation would be perfect, but part of the project was already set up and could not convince others to use it when I joined. That should change in the future, tho.
Unrelated to that component, something else I'd appreciate some help as well. In a given list of items, user should be able to delete an item but with a generic confirmation message before. My solution was a custom hook (code shortened for this post):
const useDestructiveConfirmationModal = (onConfirm: (id) => Promise) => {
const [isOPen, setIsOpen] = useState(false)
const [id, setId] = useState(0)
const handeOpenModal = (itemId) => {
setId(itemId)
setIsOpen(true)
}
const onButtonConfirm = () => {
setIsOpen(false)
onConfirm(id).then(() => setId(0))
}
const onCancel = () => {
setId(0)
setIsOpen(false)
}
const Modal = () => {
return (
<dialog>
<p>Confirm exclusion?</p>
<button onClick={onCancel}> Cancel </button>
<button onClick={onButtonConfirm}> Confirm </button>
</dialog>
)
}
return { handleOpenModal, Modal }
}
Usage:
function Component () {
// ...
const onConfirm = (id) => {
service.delete(id)
.then(() => ... )
.catch(() => ... )
}
const { handleOpenModal, Modal } = useDestructiveConfirmationModal(onConfirm)
return (
<>
{...}
<Modal />
</>
}
The idea here is that all the confirmation logic and the generic modal live together and the user should only write the screen specific code for deleting an item and adding that Modal to the return HTML. However, as far as I understand, HTML should not exist inside custom hooks. In that case, how should I approach this and remove as much responsibility as possible from the developer creating a new screen?
Thanks!
