DayPickerInput check for prop change with previous props rather than previous state #495
Conversation
kradical
changed the title
Codecov Report
@@ Coverage Diff @@
## master #495 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 15 15
Lines 531 534 +3
Branches 109 111 +2
=====================================
+ Hits 531 534 +3
Continue to review full report at Codecov.
|
src/DayPickerInput.js
Outdated
| @@ -78,21 +78,28 @@ export default class DayPickerInput extends React.Component { | |||
| } | |||
|
|
|||
| componentWillReceiveProps(nextProps) { | |||
| const { month, value } = this.state; | |||
| const hasDifferentValue = nextProps.value !== value; | |||
| const { dayPickerProps: { month }, value } = this.props; | |||
There was a problem hiding this comment.
To make the code easier to understand, I usually avoid destructuring an object at the second level: could you please here just use:
const { dayPickerProps, value } = this.props;and update the code below? Thanks!
src/DayPickerInput.js
Outdated
|
|
||
| const hasDifferentValue = nextValue !== value; | ||
|
|
||
| const monthExistsOnBoth = nextMonth && month; |
There was a problem hiding this comment.
So this would become:
const monthExistsOnBoth = nextDayPickerProps.month && dayPickerProps.month;I'd change the variable name from monthsExistsOnBoth to willUpdateDayPickerMonth (I don't understand what both are or why a month should exist or not :))
There was a problem hiding this comment.
very true, will update tomorrow!
src/DayPickerInput.js
Outdated
| const { | ||
| dayPickerProps: { month: nextMonth }, | ||
| value: nextValue, | ||
| } = nextProps; |
There was a problem hiding this comment.
Here also:
dayPickerProps: nextDayPickerProps,I know it's a bit more verbose, but it would really help!
|
Thanks! |
Hello!
I filed this issue: #494.
I have tested this with dayPickerProps and without dayPickerProps.