-
Notifications
You must be signed in to change notification settings - Fork 0
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/open one form and clear #7
Conversation
- update to React 16.3 - remove componentWillMount in exchange for new static lifestyle method getDerivedStateFromProps - feature implemented by lifting 'active' form state up to index where they are rendered - known issue - Warning received in console claiming that componentWillReceiveProps() is being called in Form component. - Form state is not cleared when clicking the same hand
- no other changes to last commit
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.
Looks really good! just those two things with the returns
@@ -3,8 +3,26 @@ import PropTypes from 'prop-types'; | |||
import firebase from 'lib/firebase'; | |||
|
|||
class Form extends Component { | |||
constructor() { | |||
super(); | |||
static getDerivedStateFromProps(nextProps, prevState) { |
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.
@bortojac I would refactor this to only have one return, I would have an object variable and then change just the hand property based on your if statement.
pages/index.js
Outdated
|
||
resetFormState(activeFormName) { | ||
// we want to update the formActive state for all hands except activeFormName | ||
const newRegions = this.state.regions.map(obj => { |
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.
Same issue as above. You're ending up writing the same thing twice, want to keep the code as DRY as possible
…esetFormState() in index.js
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.
Very nice
Take a look at the warning in the console. It appears to have no material affect. We are not calling componentWillReceiveProps in the Form component, yet the warning appears. There is still a lot of work being done on these new lifecycle methods,see here, so I'm wondering if this is a bug in 16.3.
We can always revert to 16.2 or use the UNSAFE methods until they are deprecated in 17