siemens / ix

Siemens Industrial Experience is a design system for designers and developers, to consistently create the perfect digital experience for industrial software products.
https://ix.siemens.io/
MIT License
189 stars 65 forks source link

[IxSelect] Bad interoperability with react-hook-form #691

Open robert-wettstaedt opened 1 year ago

robert-wettstaedt commented 1 year ago

What happened?

As per the docs your recommended way to validate forms is to use react-hook-form. Unfortunately, you do not provide examples how to integrate iX' custom form components with that library. After trying to integrate IxSelect with the form validation, I have found two main problems:

  1. The class form-control

In order for bootstrap to show the validation status of a form element, it requires the use of the form-control class. However, adding that class to IxSelect will draw a border around the component that looks terrible (see "iX Select" in the example):

Screenshot 2023-08-15 at 08 49 11

I could of course remove the form-control class, but then the form element's errors are not going to be displayed (see "iX Select without form-control" in the example).

  1. Standard onChange vs non-standard onItemSelectionChange event

Per default, react-hook-form's register function expects an onChange event. The IxSelect not only uses a different naming convention for the event, but also uses a non-standard way of attaching the event data to the event object by setting it to event.detail. This requires a bit of data transformation to be compatible with react-hook-form:

const { register } = useForm()
const api = register("select")

return (
  <form>
    <IxSelect
      id="select"
      onItemSelectionChange={(event) => 
        api.onChange({
          target: {
            name: "select",
             value: event.detail
           }
         })
       }
       {...api}
      />
  </form>
)

Personally, I would prefer if all form elements adhere to the standard naming convention for props, such as onChange and value instead of some arbitrary names. If that is not possible at the very least, please provide some good examples in your doc on how to integrate your components with your recommended form validation.

What type of frontend frameware are you seeing the problem on?

React

Which version of iX do you use?

v1.6.3

Code to produce this issue.

https://codesandbox.io/s/ixselect-bad-interoperability-with-react-hook-form-ft3slv
fakhir commented 10 months ago

I also faced these same problems when using IxSelect with react-hook-form. One suggestion for the sample code you've mentioned above would be to embed IxSelect within the react-hook-form's Controller element, which gives better integration with "value" and defaults, though it's not ideal like you mention:

  <Controller
    name="appType"
    control={control}
    rules={{ required: true }}
    render={({ field: {name, onChange, onBlur, value, ref} }) => (
      <IxSelect id="inputAppType" name={name} onItemSelectionChange={(data) => onChange(data.detail[0])} selectedIndices={value} onBlur={onBlur}>
        ...
      </IxSelect>
    )}
  />
danielleroux commented 10 months ago

How @fakhir descibe is the way how to integrate web components inside react-form-hooks.

As soon as the stencil team merged the following pr https://github.com/ionic-team/stencil/pull/4784 we can start to work also to enable native web components support of our components.

danielleroux commented 10 months ago

Stencil released v4.5 which support web components in combination with a form. Improvements regarding components and form is planned in our internal backlog [IX-561]