Skip to content

Conversation

Hyperkid123
Copy link
Member

closes #189

Day picker can now accept value as a string.

@Hyperkid123 Hyperkid123 requested review from skateman and rvsia November 1, 2019 08:41
@Hyperkid123 Hyperkid123 force-pushed the fix-date-picker-initial-value branch from a3cbe3a to 43a0857 Compare November 1, 2019 08:43
Copy link
Contributor

@rvsia rvsia left a comment

Choose a reason for hiding this comment

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

@Hyperkid123 Could you please write a little test? Thanks! 🌮

@Hyperkid123
Copy link
Member Author

ups forgot add the WIP thing Working on it right now.

@Hyperkid123 Hyperkid123 changed the title fix(pf3): stop date picker crash when value is string. [WIP] fix(pf3): stop date picker crash when value is string. Nov 1, 2019
@codecov-io
Copy link

codecov-io commented Nov 1, 2019

Codecov Report

Merging #191 into master will increase coverage by 1.28%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #191      +/-   ##
==========================================
+ Coverage   81.74%   83.03%   +1.28%     
==========================================
  Files          87       87              
  Lines        1397     1397              
  Branches      344      344              
==========================================
+ Hits         1142     1160      +18     
+ Misses        211      197      -14     
+ Partials       44       40       -4
Impacted Files Coverage Δ
...c/form-fields/date-time-picker/date-time-picker.js 64.7% <ø> (+52.94%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7eb78d3...7a347a9. Read the comment docs.

@skateman
Copy link
Member

skateman commented Nov 1, 2019

Now it's throwing me Invalid Date if the initialValue is not set:

{
  "component": "date-picker",
  "name": "date-picker-1",
  "label": "Datepicker 1",
  "visible": true,
  "disabledDays": [
    {
      "before": "today"
    }
  ],
  "variant": "date"
}

Screenshot from 2019-11-01 10-15-29

And by clicking on the picker icon, it crashes with Cannot read property 'length' of undefined

@Hyperkid123
Copy link
Member Author

@skateman ah there is one mor thing i forgot

@Hyperkid123 Hyperkid123 force-pushed the fix-date-picker-initial-value branch from 87a5abf to 7a347a9 Compare November 1, 2019 09:22
@Hyperkid123 Hyperkid123 changed the title [WIP] fix(pf3): stop date picker crash when value is string. fix(pf3): stop date picker crash when value is string. Nov 1, 2019
Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

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

The Seal of Approval

@Hyperkid123 Hyperkid123 requested a review from rvsia November 1, 2019 09:44
@rvsia
Copy link
Contributor

rvsia commented Nov 1, 2019

@Hyperkid123 what about to add a test that proves that picker icon click crash is gone, just to be sure it won't be repeated in the future? Otherwise looks good!

@Hyperkid123
Copy link
Member Author

@rvsia the crash has nothing to do with the clicl but with the fact the it was sending string to the day picker component instead of Date object.

@rvsia rvsia merged commit 544aca3 into master Nov 1, 2019
@Hyperkid123 Hyperkid123 deleted the fix-date-picker-initial-value branch November 1, 2019 09:54
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.

Opening a datepicker with a preset initialValue crashes
4 participants