Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(DatePicker): explicitly update minDate and maxDate #4912

Merged
merged 4 commits into from
Jan 7, 2020
Merged

fix(DatePicker): explicitly update minDate and maxDate #4912

merged 4 commits into from
Jan 7, 2020

Conversation

figfofu
Copy link
Contributor

@figfofu figfofu commented Dec 18, 2019

Since flatpickr does not automatically update minDate and maxDate,
we need to explicitly do it when the user makes any change in the props
by using set(option, value) method from flatpickr

Fixes #2500

Changelog

New

Changed

  • explicitly update minDate and maxDate when they are changed

Removed

Testing / Reviewing

Testing should make sure is not broken.

@figfofu figfofu requested a review from a team as a code owner December 18, 2019 05:45
@ghost ghost requested review from asudoh and joshblack December 18, 2019 05:45
asudoh
asudoh reviewed Dec 18, 2019
View reviewed changes
Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

Hi 👋 @figfofu thank you for jumping in! Any change to align this change to #4856? Thanks!

@netlify
Copy link

netlify bot commented Dec 18, 2019 •
edited
Loading

Deploy preview for the-carbon-components ready!

Built with commit fe9af26

https://deploy-preview-4912--the-carbon-components.netlify.com

@figfofu
Copy link
Contributor Author

figfofu commented Dec 18, 2019

I think we can do that.
I was thinking of making the change to either componentDidUpdate() or componentWillUpdate(),
and then decided to add to the existing method componentWillUpdate().

I can wait for #4856 getting merged then add the change.

@netlify
Copy link

netlify bot commented Dec 18, 2019 •
edited
Loading

Deploy preview for carbon-elements failed.

Built with commit fe9af26

https://app.netlify.com/sites/carbon-elements/deploys/5e1504a58efe320008f2224b

@netlify
Copy link

netlify bot commented Dec 18, 2019 •
edited
Loading

Deploy preview for carbon-components-react ready!

Built with commit fe9af26

https://deploy-preview-4912--carbon-components-react.netlify.com

Bui Duc Binh added 2 commits January 7, 2020 13:11
fix(DatePicker): explicitly update minDate and maxDate
28fc801 
Since flatpickr does not automatically update minDate and maxDate,
we need to explicitly do it when the user makes any change in the props
by using set(option, value) method from flatpickr
fix(DatePicker): explicitly update minDate and maxDate
cc80a71 
Since flatpickr does not automatically update minDate and maxDate,
we need to explicitly do it when the user makes any change in the props
by using set(option, value) method from flatpickr
Aligned to the change in carbon-design-system#4856
@figfofu figfofu force-pushed the date_picker_update_minDate_maxDate branch from d14d293 to cc80a71 Compare January 7, 2020 05:03
@figfofu figfofu requested a review from asudoh January 7, 2020 05:08
@asudoh
Copy link
Contributor

asudoh commented Jan 7, 2020

Thank you for the update @figfofu! Going through props with an array is an interesting approach, but I'd rather not abstracting that... Would it be OK for you to add the logic for minDate/maxDate to the existing cDU?

BTW you can simply add comments once you add update commits and I'll get notified via email.

fix(DatePicker): explicitly update minDate and maxDate
fc786c7 
Since flatpickr does not automatically update minDate and maxDate,
we need to explicitly do it when the user makes any change in the props
by using set(option, value) method from flatpickr
Aligned to the change in carbon-design-system#4856
@figfofu
Copy link
Contributor Author

figfofu commented Jan 7, 2020

@asudoh
Updated the change.
Please review it.

asudoh
asudoh approved these changes Jan 7, 2020
View reviewed changes
Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - Thanks @figfofu!

@asudoh asudoh merged commit ef0752e into carbon-design-system:master Jan 7, 2020
joshblack pushed a commit to joshblack/carbon that referenced this pull request Jan 13, 2020
@figfofu @joshblack
fix(DatePicker): explicitly update minDate and maxDate (carbon-design…
f6f040e 
…-system#4912)

Since flatpickr does not automatically update minDate and maxDate,
we need to explicitly do it when the user makes any change in the props
by using set(option, value) method from flatpickr
@joshblack joshblack mentioned this pull request Jan 13, 2020
Closed
joshblack pushed a commit to joshblack/carbon that referenced this pull request Jan 14, 2020
@figfofu @joshblack
fix(DatePicker): explicitly update minDate and maxDate (carbon-design…
7315830 
…-system#4912)

Since flatpickr does not automatically update minDate and maxDate,
we need to explicitly do it when the user makes any change in the props
by using set(option, value) method from flatpickr
@joshblack joshblack mentioned this pull request Jan 14, 2020
Closed
@joshblack joshblack mentioned this pull request Jan 31, 2020
Closed
@joshblack joshblack mentioned this pull request Feb 25, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshblack joshblack joshblack approved these changes

@asudoh asudoh asudoh approved these changes

Assignees
No one assigned
Labels
status: ready to merge 🎉
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Min and Max dates in calendar of datapicker are not updated upon component update
3 participants
@figfofu @asudoh @joshblack