Availity / availity-react

React components using Availity UIKit and Bootstrap 4
https://availity.github.io/availity-react
MIT License
54 stars 30 forks source link

formik Allow Value Props for overrides #359

Open GoPro16 opened 5 years ago

GoPro16 commented 5 years ago

🚀 Feature request

Current Behavior

All formik inputs, selects, and dates don't allow value props to be passed in like availity-reactstrap-validation does. Maybe its time we add this in here or we should kill this issue with fire and re-open later if we think our opinion changes.

Desired Behavior

When a value is passed in as a prop to <Input value="hello" /> that value should override the one provided from formik and also update the current value if they are not equal.

Suggested Solution

Add effect logic that takes the value from the props and compares it to the formik field value and if different, updates the field value for that input.

Who does this impact? Who is this for?

All users wanting to manage field state in HOCs etc.

Describe alternatives you've considered

As an alternative, if one field is dependent on the output of another, you can use the setFieldValue from useFormikContext in order to achieve this.

Additional context

We shouldn't have to worry about putting extra renders on the component. At most it will be 1 more extra render as we are updating the field value of the component once and then it should re-render without calling the validation logic.

jordan-a-young commented 5 years ago

The use case I come across often is one field depending on another. I like the alternative idea you proposed, but it would be nice if there was a prop available for that. Either one that accepts a function or object and changes depending on another field OR a prop that will change another field's value based on this field.

nylon22 commented 5 years ago

Allowing a value prop to be passed down enables a bad pattern IMO. I think we want to encourage managing the state of the form with formik hooks as opposed to having that state stored elsewhere. formik makes it easy to manage that state and it doesn't make sense to me to have that state stored in two separate places - and to have to worry about keeping it in sync.

Perhaps a value props makes it easier to manage state with class components. but i think everyone is making the switch to functional components.

andrectoi commented 5 years ago

I see 2 patterns here: 1) Fields behave like listeners that 'subscribe' to particular field updates. 2) The field that changes dictates or 'propagates' necessary changes to the other fields it chooses.

I agree that we need to stick to one pattern instead of allowing two patterns because it can cause people's code to be messy in the long run, especially if two developers have different approaches to managing form state when doing maintenance.

TheSharpieOne commented 5 years ago

allowing value to be set makes using Input more similar to React, which makes it easier since it work just work as expected. But, I agree that allowing both is probably not great. If someone adds value to Input, I would throw an error in dev environment warning the user that it doesn't work and instructing them (directly or via a link to the docs) how to properly change the value of the input.

GoPro16 commented 5 years ago

Maybe we start a dev blog and link to it like https://kentcdodds.com/blog and include examples on how to implement that sort of logic properly.

availity.github.io/dev-blog 🚀 ❓