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.

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

chore: added minDate and maxDate checks in the datepicker #3060

Merged
merged 6 commits into from
Nov 25, 2024

Conversation

rkkp1023
Copy link
Contributor

@rkkp1023 rkkp1023 commented Nov 15, 2024

Closes #3059

This PR will add the minDate and maxDate to the flatpicker options for the story book

Changelog

New

image

@rkkp1023 rkkp1023 requested a review from a team as a code owner November 15, 2024 06:38
Copy link

netlify bot commented Nov 15, 2024

Deploy Preview for carbon-components-angular ready!

Name Link
🔨 Latest commit c428711
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-angular/deploys/6744923bc4ba840008623b0b
😎 Deploy Preview https://deploy-preview-3060--carbon-components-angular.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@rkkp1023 rkkp1023 changed the title fix: added minDate and maxDate in the datepicker fix: added minDate and maxDate checks in the datepicker Nov 15, 2024
Comment on lines 229 to 238
/**
* The minimum date that a user can start picking from.
*/
@Input() minDate: string | number;

/**
* The maximum date that a user can pick to.
*/
@Input() maxDate: string | number;

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @rkkp1023, this was intentionally excluded. At the time of the component creation, it was decided that we weren't going to create explicit wrapper inputs due to the possibility of changes upstream (flatpickr). If we start adding flatpickrOptions as inputs, we will then need to add ALL.

Instead, please pass the maxDate/minDate as part of the flatpickrOptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @Akshat55, Added flatpickerOptions in the storybook

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Akshat55, Q: why the Carbon Angular implementation needs to differ from other implementations - both React and Web Component implementations support minDate/maxData as input properties.

Copy link
Contributor

Choose a reason for hiding this comment

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

We tend to follow React implementation for the most part, however there are certain components that we decided to not 100% follow in terms of implementation. Date picker is one of those components.

Date picker is dependent on a third-party library (flatpickr), so it doesn't make sense for us to add all of those inputs/outputs in case there is a change in flatpickr.

In such case, to resolve this, we would need to phase it out via breaking changes, so the reasonable thing to do on our end was allow for a user to pass in a flatpickr options object.

That being said, I definitely think we do need to improve our storybook documentation to better showcase this.

Akshat55
Akshat55 previously approved these changes Nov 25, 2024
src/datepicker/datepicker.component.ts Outdated Show resolved Hide resolved
@Akshat55 Akshat55 changed the title fix: added minDate and maxDate checks in the datepicker chore: added minDate and maxDate checks in the datepicker Nov 25, 2024
@Akshat55 Akshat55 merged commit 6febdbd into carbon-design-system:master Nov 25, 2024
14 checks passed
Copy link

🎉 This PR is included in version 5.56.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

[ Enhancement ] : Add minDate and maxDate check in the Datepicker
2 participants