Open lauri865 opened 1 year ago
Imagine a use case of data importing (e.g. from CSV file):
Might seem like a long list of requirements, but I can see a very easy and clean path to implementing this with e.g. react-hook-form with minimal amount of code and within a very short timeframe, since they have all the mechanisms for field state tracking, unloading fields, field rules + schema based validation, resetting the form as well as individual fields, etc.
On the other hand, trying to fit this into the Datagrid's internal "form" API, I'm envisioning an obscene amount of code and loads of manual data transformations to derive states, resetting, etc., and it's bound to be more error-prone, as I need to introduce more moving parts.
After playing around a bit, I think something like useCellParams
or useCellProps
could be a good API to have. And use
rather than get
is to explicitly support hooks (e.g. useFormContext()). This could enable registering the fields dynamically, without having to introduce additional custom components, and pass back e.g. onChange events to renderers if in editing mode and className for cell to optionally style cells if they have errors or have changes.
I think I can achieve it in user land by creating a custom wrapper around Cell
component for editable cells. Will report back how it goes. And happy to be directed to a better way to achieve this.
Update: I cannot use a custom wrapper around GridCellV7. The following line doesn't recognize that the slot is rendering a GridCellV7 already, so it adds GridCellWrapper around it: https://github.com/mui/mui-x/blob/8b67c900ae9904a7361e3eff887dccbc53b4d565/packages/grid/x-data-grid/src/components/GridRow.tsx#L269
@lauri865 Thanks for your suggestions, some really good points here!
I'm adding the waiting for upvotes
label to check how the community resonates with it, but there is definitely room for improvements when it comes to the Editing API.
I played around with this over the weekend to interface with react-hook-form
and zod
and got it working pretty well, even with paste-support. However, I ran into a roadblock when looking into virtualization and that is where react-hook-form doesn't fit too well because it relies on a rendered form. The effect of it is that the form resets its defaultValues
when scrolling forth and back such that the virtualization kicks in.
Value parsing also ends up being redundant, seeing that the resolver
and columns' valueParser
do the same thing.
If anyone's interested, these are the two components that make things possible.
const ValidatedRow: FC<
GridRowProps & {
onSubmit: (form: FieldValues) => Promise<void>;
resolver: ReturnType<typeof zodResolver>;
}
> = ({ resolver, onSubmit, ...props }) => {
const formMethods = useForm({
mode: 'onChange',
defaultValues: props.row,
resolver,
});
const [isSaving, setSaving] = useState(false);
const submit = formMethods.handleSubmit(async (form) => {
if (!formMethods.formState.isDirty) {
return;
}
setSaving(true);
await onSubmit(form);
setSaving(false);
});
return (
<form onSubmit={submit}>
<FormProvider {...formMethods}>
<GridRow {...props} />
</FormProvider>
</form>
);
};
const ValidatedCell: FC<GridCellProps> = (props) => {
const apiRef = useGridApiContext();
const {
formState: { errors },
control,
getValues,
setValue,
} = useFormContext();
const name = props.field as keyof FieldValues;
const { classes, cx } = useStyles();
const column = apiRef.current.getColumn(name);
useEffect(() => {
const fieldValue = getValues(name);
const value = column.valueParser ? column.valueParser(fieldValue) : fieldValue;
// Upon pasting, the DOM input field is not rendered and no onChange event happens.
const hasValueChangedOutsideOnChange = value !== props.value;
if (hasValueChangedOutsideOnChange) {
setValue(name, props.value, { shouldValidate: true, shouldDirty: true });
}
}, [props.value]);
return (
<Controller
render={({ field }) => (
<GridCell {...props} {...field} className={cx(props.className, { [classes.error]: name in errors })} />
)}
name={name}
control={control}
/>
);
};
...
return (
<DataGridPremium
apiRef={apiRef}
experimentalFeatures={{ clipboardPaste: true }}
unstable_cellSelection
disableRowSelectionOnClick
columns={columns}
slots={{ cell: ValidatedCell, row: ValidatedRow }}
slotProps={{
row: {
resolver,
onSubmit: submitRow,
},
}}
rows={rows}
/>
);
Did you make sure to disable shouldUnregister? If shouldUnregister is enabled, it clears the value on unmount, but retains it otherwise.
Did you make sure to disable shouldUnregister? If shouldUnregister is enabled, it clears the value on unmount, but retains it otherwise.
It's actually related to defaultValues
being set to props.row
when the form context is re-initialized and the reference to the original (non-dirty) state is lost. Setting shouldUnregister
to false has no effect here.
Right, didn't check your code. I wrapped the whole datagrid in form context, rathet than individual rows. Have more learnings on this, but limited on time right now to share.
@martinjlowm your snippet is awesome and helped me greatly. Absolute kudos for that
slots={{row: ValidatedRow}}
covers base case when using custom cells, woo!
ValidatedCell
doesnt seem to jive if need the control
in custom input columns via renderCell
For recipe request: an easy way to keep the DataGrid state and form state in sync would be awesome.
apiRef
's updateRows([{...row, id: props.row.id}])
can handle it, but perhaps an onCellChange
within the recipe would simplify?
End result might be exposing the params.api.onCellChange
to work with a control
from a form lib?
Regardless, the dual-state seems the challenge.
From UX perspective, DataGrid's current paradigm of double click or toggle Edit mode per Cell is kind of clunky when using it as an input Grid.
Here is a demo https://codesandbox.io/p/sandbox/dv48jd There is an issue with react-hook-form when you are deleting items tho
Summary 💡
Both relating to docs as well as improvements in the API to enhance developer experience. Related to #4009, but covering the whole of form logic – validation, submission and everything in between, not just simply validation.
The thing I like by far the least about the datagrid is the editing API. Not because of any particular design decision on your part, but just the fact that it essentially forces me to duplicate form logic specifically for the datagrid, and I need to study a custom API for validations, parsing, changing values, etc.
I would much rather if datagrid didn't touch any of these topics (by default) beyond controlling the representation of the form fields (are the form fields in editing mode or not). Any reasonable application already implement form logic elsewhere and there are well established form libraries that cover all the topics around validation, parsing, etc.
It'd be much better DX if the docs suggested a recipe for BYOFL – bring your own form library, as well as good APIs to connect existing fields to a form, and pass back validation state to the fields (validation errors, etc.) without having to create custom edit components for all fields (which to me seems the current best way to achieve this). The logic for controlling inputs is overlapping across all field types, it would suffice just to have an ability to register the field name, and pass back onChange, onBlur, isValid and errorMessage. These are fairly standard across any form library, and I want to emphasise that I'm not asking for an integration with any specific form library, but rather a bridge to easily connect datagrid with any form library.
I think it would make it much easier to implement editable datagrids, reduce the amount of redundant code / duplicate logic in apps, as well as reduce the amount of Datagrid API surface area on needs to study considerably.
Examples 🌈
No response
Motivation 🔦
The Data Grid - Editing docs page is 3,000+ words of content to consume, which can easily be overwhelming, while one could be up an running with their existing form library in 5 minutes tops, and reuse existing validations, parsers, etc. they use elsewhere in their app.
Search keywords: form, react-hook-form