-
Notifications
You must be signed in to change notification settings - Fork 45
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
Creators auto-complete #410
Conversation
formikProps.form.setFieldValue(path, value); | ||
}); | ||
// Update identifiers render | ||
this.identifiersRef.current.setState({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From a quick look (but I am probably wrong), in general this is a sign that the state should be handled by the upper component.
If you remove the state selectedOptions
from CreatibutorsIdentifiers
, and you manage it in this cmp, you should have here:
this.setState({selectedIdentifer: ....})
in the render method, you pass it as a prop, so that CreatibutorsIdentifiers
will re-render with this new selectedIdentifier
. You will also handle the callbacks and formik set in the upperCmp: the inner one will become very dummy.
If the logic in this upper becomes very messy, you can always introduce a middle cmp, between the 2, where the middle one encapsulates/manages the state of the inner one and the upper one is a bit lighter...
I had to do something similar in the PIDs cmp.
Happy to chat IRL :) I hope it helps (even if I am very positive that you have already thought about this).
Squash-merged manually in 04eba96 (but GitHub doesn't detect that...) |
Changes introduced from the creatibutors auto-complete feature branch. Some notes for the reviewer:
(Closes #397)