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

feat(ui5-datepicker): implementing min and max date limits #1040

Merged
merged 62 commits into from
Jan 31, 2020
Merged

feat(ui5-datepicker): implementing min and max date limits #1040

merged 62 commits into from
Jan 31, 2020

Conversation

tsanislavgatev
Copy link
Contributor

Hello team, can you please review this PR.

Regards,
Tsanislav

if (this._minDate) {
this.timestamp = this._minDate / 1000;
} else {
this.timestamp = (new Date(-62135596800000)).getTime() / 1000;
Copy link
Contributor

Choose a reason for hiding this comment

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

Magic number. Please use a constant that explains what this is

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}

event.preventDefault();

await RenderScheduler.whenFinished();
Copy link
Contributor

Choose a reason for hiding this comment

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

This was meant to be "unofficial" - for the tests only. I'm fine for the moment, but we need to provide a public function for the same.
@pskelin

*/
minDate: {
type: String,
defaultValue: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary to set "undefined" here? Isn't it undefined by default (or is it empty string)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's an empty string, but still a falsy value so it can be removed, thanks.

*/
maxDate: {
type: String,
defaultValue: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

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

Successfully merging this pull request may close these issues.

None yet

5 participants