Skip to content
This repository has been archived by the owner on Apr 27, 2022. It is now read-only.

Add default name to form components #147

Merged

Conversation

Nevon
Copy link
Member

@Nevon Nevon commented Oct 25, 2016

Fixes #67

So this is a first attempt at generating stable, unique names for form controls where the name is not specified as a prop.

I've only added it to the uncontrolled field at the moment, so to try it out, remove the name from one of the examples where it is used. When the control re-renders, the name should stay the same, and the name should be unique.

The general idea is that we concatenate the component name with a UUIDv4, which is seeded using a counter in the closure. This means that the uuid will very likely be re-used, but only with other components. There should never be a complete name collision due to the component name being different.

@xaviervia
Copy link
Contributor

OK, finally got the chance to go back to this one.

Love it. Perfect approach. I will merge the "make everything uncontrolled" branch into this and put the higher-order component in all the relevant components, I think we’ll be good to go.

@xaviervia xaviervia changed the title WIP: Add default name to form components Add default name to form components Nov 11, 2016
@xaviervia
Copy link
Contributor

Works now. @Nevon mind to review?

@xaviervia xaviervia changed the base branch from master to make-uncontrolled-the-default November 11, 2016 15:09
Copy link
Member Author

@Nevon Nevon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird to be a reviewer of my own PR...

Other than the leftover files that shouldn't be needed, it looks good to me.

@@ -0,0 +1,13 @@
import seededRandom from 'seed-random'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be needed anymore, right, since you're using the higher-order-components package instead. Same for lib/uniqueComponentName.

@Nevon
Copy link
Member Author

Nevon commented Nov 17, 2016

And now the build is failing because there's an imprecise dependency of react. react-maskedinput seems to be importing a private module from react, which either no longer exists or has been moved. insin/react-maskedinput#76

@Nevon Nevon force-pushed the add-default-name-to-form-components branch from c2b5190 to 53493ae Compare November 22, 2016 13:44
@Nevon Nevon merged commit a19f997 into make-uncontrolled-the-default Nov 22, 2016
@Nevon Nevon deleted the add-default-name-to-form-components branch November 22, 2016 13:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants