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

fix(core): handle invalid date inside date's components #5851

Merged
merged 1 commit into from
Jul 3, 2021

Conversation

Smiranin
Copy link
Contributor

@Smiranin Smiranin commented Jun 30, 2021

Please provide a link to the associated issue.

Closes #5059

Please provide a brief summary of this pull request.

This request fixed this issues(read point 3) stackblitz.
What was done:

  • Implemented standard behavior(as it works for Date calss of native JS or moment.js) for invalid date. Now we don't get error exception if we pass invalid date during creating instance of FdDate class. If FdDate instance contains invalid date, we get NaN value for all number formats and string message "Invalid Date" for string format.
  • Adapted date's components for new behavior
  • Fixed several bugs of date's components.

BREAKING CHANGES:

FdDate class invalid date handling:

  • Before:
    You set invalid date params -> You are getting throwing error.
  • After:
    You set invalid date params -> You are getting an invalid date instance.

DateTimePickerComponent invalid date handling:

  • Before:
    You type in input invalid date format -> You are getting date instance with current date.
  • After:
    You type in input invalid date format -> You are getting an invalid date instance.

DatePickerComponent invalid date handling:

  • Before:
    You type in input invalid date format -> You are getting null
  • After:
    You type in input invalid date format -> You are getting an invalid date instance.

Please check whether the PR fulfills the following requirements

Documentation checklist:

@Smiranin Smiranin requested a review from dimamarksman June 30, 2021 14:08
@netlify
Copy link

netlify bot commented Jun 30, 2021

✔️ Deploy Preview for fundamental-ngx ready!

🔨 Explore the source changes: 43b8e1f

🔍 Inspect the deploy log: https://app.netlify.com/sites/fundamental-ngx/deploys/60debf24c1c8060007cedeb7

😎 Browse the preview: https://deploy-preview-5851--fundamental-ngx.netlify.app

libs/core/src/lib/datetime/fd-date.ts Show resolved Hide resolved
libs/core/src/lib/datetime/fd-date.ts Show resolved Hide resolved
libs/core/src/lib/datetime/fd-date.ts Outdated Show resolved Hide resolved
libs/core/src/lib/datetime/fd-date.ts Outdated Show resolved Hide resolved
libs/core/src/lib/datetime/fd-date.ts Show resolved Hide resolved
libs/core/src/lib/utils/consts/error-messages.ts Outdated Show resolved Hide resolved
@InnaAtanasova InnaAtanasova added the bug Something isn't working label Jun 30, 2021
@InnaAtanasova InnaAtanasova added this to the Sprint 65 - Bern milestone Jun 30, 2021
@InnaAtanasova
Copy link
Contributor

The checkboxes in the PR template should be all checked or marked NA as the Mergeable check will always fail.
Dima's comments need to be addressed, the rest looks good.

@Smiranin Smiranin force-pushed the datepicker-entering-validation branch 4 times, most recently from 2eceea5 to fc37ccc Compare July 1, 2021 15:37
@Smiranin Smiranin force-pushed the datepicker-entering-validation branch from fc37ccc to 43b8e1f Compare July 2, 2021 07:24
Copy link
Contributor

@dimamarksman dimamarksman left a comment

Choose a reason for hiding this comment

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

Looks good.
The only thing that we are introducing breaking changes here since now instead of throwing error we are getting an invalid date instance.

Please add corresponding Breaking Changes description in the PR

@droshev droshev merged commit f1f3705 into main Jul 3, 2021
@droshev droshev deleted the datepicker-entering-validation branch July 3, 2021 02:22
maxym-valor pushed a commit to maxym-valor/fundamental-ngx that referenced this pull request Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Date-/Datetime-/Time-picker: format-value validation cannot be checked and not normal behaviours
5 participants