osuresearch / ui

Ohio State Research UI
https://osuresearch.github.io/ui/main
MIT License
6 stars 3 forks source link

Reset button doesn't work in form examples #59

Closed helloLilyX closed 1 year ago

helloLilyX commented 1 year ago

Clicking on the button doesn't clear the form

McManning commented 1 year ago

Can confirm that this happens to RHF 7 as well. Internal state resets but not the visuals.

This one is hard to solve, there's some juggling that has to happen between what RHF wants to manage and what React Aria wants to manage. I don't want to make everything controlled components for integrating with RHF though.

McManning commented 1 year ago

So, it's a tossup on how I want to solve this.

Since register from RHF is for uncontrolled components, it only ever sets defaultValue and not value. The reset will update defaultValue but it won't trigger any re-renders (which is working as intended for uncontrolled components).

I'd then only be able to suggest using RHF in uncontrolled mode under the condition that:

  1. The form is rendered after the RHF defaultValues are set from wherever they need to be set or 2. A key prop is used to force form components to re-mount after RHF reset happens.

Option 2 is inspired by this blog post about uncontrolled component resets.

With that said, the most common method people seem to suggest is to use RHF's Controller or useController to wrap and bind the fields in controlled mode - thus reset will ... well... control them. I still think their API is pretty awkward for this and I hate the idea of wrapping every field in another component as a render prop. But with the magic of polymorphic components in RUI 5 - we can simplify this down pretty easily to something like:

export const ControlledField = polymorphicForwardRef<any, ControlledFieldProps>(
  ({ as, name, ...props }, ref) => {
    const { control } = useFormContext();
    const { field } = useController({
      name,
      control
    });

    const { ref: fieldRef, ...otherFieldProps } = field;

    return (
      <Box ref={mergeRefs(ref, fieldRef)} as={as || 'input'} {...props} {...otherFieldProps} />
    )
  }
)

And then usage would be:

<ControlledField as={TextField} name="email" label="Controlled email" /> 

Would still need to add various props in there for supporting RHF's validation pieces but it's a start of a potential option.

Still thinking of some alternatives for this. Since I'm already wrapping RHF's useForm with my own useRUIForm to do data transformations of more complex FormField types, I could just expose a controlled variant of register that spreads defaultValue, value, onChange, onBlur, name into the form fields.

Anyway, the tl;dr is that the current demo of RHF 7 is "working as intended". But I need to split it off into an uncontrolled RHF demo (and documenting the gotcha's you'd encounter for populating an uncontrolled form) + a controlled RHF demo.

github-actions[bot] commented 1 year ago

:tada: This issue has been resolved in version 5.0.0-alpha.22 :tada:

The release is available on:

Your semantic-release bot :package::rocket: