Skip to content
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

[Feature Request]: Add support for react-hook-form #414

Closed
2 tasks done
ab18556 opened this issue Oct 26, 2022 · 4 comments
Closed
2 tasks done

[Feature Request]: Add support for react-hook-form #414

ab18556 opened this issue Oct 26, 2022 · 4 comments
Labels
enhancement New feature or request

Comments

@ab18556
Copy link

ab18556 commented Oct 26, 2022

Description

The way the ref is used on each elements at the moment we cannot easily use react-hook-form. For component libraries, react suggest that we forward the ref.
https://reactjs.org/docs/forwarding-refs.html

react-form-hook use the ref to bind the events on the input itself, but with the actual state of the ref it doesn't work.
https://react-hook-form.com/api/useform/register#registerRef

There is a workaround, but it makes react-form-hook less usefull and fun to work with.
https://react-hook-form.com/faqs#Whatifyoudonthaveaccesstoref

react-form-hook is a really popular library used by a lot of people. Maybe there is an other work around I am not aware of. If this is the case I would be happy to have it.

Code of Conduct

@ab18556 ab18556 added the enhancement New feature or request label Oct 26, 2022
@just-boris
Copy link
Member

This can be implemented in userland

import React from 'react';
import { Controller } from 'react-hook-form';
import { Input } from '@cloudscape-design/components';

export const ControlledInput = ({ name, control, ...props }) => (
  <Controller
    name={name}
    control={control}
    render={({ field: { ref, onChange, onBlur, value } }) => (
      <Input ref={ref} name={name} value={value} onBlur={onBlur} onChange={e => onChange(e.detail.value)} {...props} />
    )}
  />
);

You can wrap every control and publish it is a separate package

Having it built into library does not sound future proof, because what if somebody wants to use another form library

@ab18556
Copy link
Author

ab18556 commented Oct 28, 2022

I understand what you mean, but at the same time I think that Cloudscape should expose standard properties as much as possible. For example, the onChange is not providing the usual react event. It might bring a lot of issues with other libraries aswell. If Cloudscape is about design, why is it fiddling with the events and refs? why is value required even if it is empty?

@just-boris
Copy link
Member

What you are asking sounds as a breaking change, but we are not planning on publishing a new major version in near future.

if you have any suggestion how to change API in a non-breaking way that may help you – let us know

@just-boris
Copy link
Member

Closing because this feature is not applicable to our project

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants