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

initialMonth should not update after first mounted #169

Closed
willmcclellan opened this Issue May 25, 2016 · 6 comments

Comments

3 participants
@willmcclellan

willmcclellan commented May 25, 2016

I'm not sure if this is the intended behaviour, but I expected the initialMonth property to only be relevant on the first mount of the component. So you set an initialMonth but subsequent renders ignore it.

The current behaviour here:

https://github.com/gpbl/react-day-picker/blob/master/src/DayPicker.js#L80-L84

... updates the current month to the initialMonth when receiving props which, if you aren't storing/updating the current month in state when clicking next/prev, means the months don't switch if initialMonth is always the same value.

If that is the intended behaviour, then maybe the naming of initialMonth should be changed as it doesn't fit with how it actually works?

@gpbl

This comment has been minimized.

Show comment
Hide comment
@gpbl

gpbl May 25, 2016

Owner

Yes I believe you are right, implementers should use the showMonth instance method instead. I think this is a leftover from some early release, but it may lead to some unintended effects.

Owner

gpbl commented May 25, 2016

Yes I believe you are right, implementers should use the showMonth instance method instead. I think this is a leftover from some early release, but it may lead to some unintended effects.

@davidspiess

This comment has been minimized.

Show comment
Hide comment
@davidspiess

davidspiess Feb 22, 2017

I'm not happy about that change. How would i now be able to (re)render the calendar, when my date variable in my store changes?

davidspiess commented Feb 22, 2017

I'm not happy about that change. How would i now be able to (re)render the calendar, when my date variable in my store changes?

@gpbl

This comment has been minimized.

Show comment
Hide comment
@gpbl

gpbl Feb 22, 2017

Owner

@davidspiess you can use the ref prop and the this.daypicker.showMonth(newMonthFromProps) in componentDidUpdate. Would that work for you?

Owner

gpbl commented Feb 22, 2017

@davidspiess you can use the ref prop and the this.daypicker.showMonth(newMonthFromProps) in componentDidUpdate. Would that work for you?

@davidspiess

This comment has been minimized.

Show comment
Hide comment
@davidspiess

davidspiess Feb 23, 2017

@gpbl yes it works that way, but i think it should be a prop like it was. When my state changes, i want my UI to reflect that. With this behavior have to call a method when this is the case, which goes against the whole purpose of react.

davidspiess commented Feb 23, 2017

@gpbl yes it works that way, but i think it should be a prop like it was. When my state changes, i want my UI to reflect that. With this behavior have to call a method when this is the case, which goes against the whole purpose of react.

@gpbl

This comment has been minimized.

Show comment
Hide comment
@gpbl

gpbl Feb 23, 2017

Owner

@davidspiess the problem lies in the prop's name: people expect initialMonth to work just for the first render (as reported by @willmcclellan here), similarly to the defaultValue in uncontrolled components.

I think i should add a new month prop behaving like the previous initialMonth - what do you think?

Owner

gpbl commented Feb 23, 2017

@davidspiess the problem lies in the prop's name: people expect initialMonth to work just for the first render (as reported by @willmcclellan here), similarly to the defaultValue in uncontrolled components.

I think i should add a new month prop behaving like the previous initialMonth - what do you think?

@davidspiess

This comment has been minimized.

Show comment
Hide comment
@davidspiess

davidspiess Feb 23, 2017

That would be awesome! 👍

davidspiess commented Feb 23, 2017

That would be awesome! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment