Skip to content

Add highlighted and disabled classes #66

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

Merged
merged 3 commits into from
Jan 15, 2021

Conversation

mst101
Copy link
Contributor

@mst101 mst101 commented Jan 11, 2021

Hi @MrWook, Happy New (calendar) Year!

In an effort to simplify the code, I made the following changes for your consideration:

  • Created two new classes (DisabledDate.js and HighlightedDate.js) in utils to determine whether a particular cell should be disabled and/or highlighted.

  • Both classes expose a config property which easily allows us to determine whether a disabled/highlighted configuration exists and if so, whether it has from, to, specificDates, daysOfMonth, daysOfWeek, customPredictor etc. properties. These boolean properties clean up quite a lot of code in other places. e.g. if (!this.disabledDates || !this.disabledDates.to) becomes if (!this.disabledConfig.has.to).

  • The config properties also contain a to and from object for easy access to e.g. this.disabledConfig.to.year as opposed to this.utils.getFullYear(this.disabledDates.to)

  • Given that highlighted dates are uniquely relevant to PickerDay.vue, I added a highlightedConfig computed property ONLY for that component and a disabledConfig computed property to pickerMixin as it's used in all 3 pickers.

  • To improve readability, I added a pageMonth computed property to PickerDay and a pageYear computed property to pickerMixin. This means we can write e.g. this.pageMonth instead of this.utils.getMonth(this.pageDate).

  • Added a test:debug script to package.json to help with debugging tests via Chrome dev tools.

  • Corrected some December dates in highlightedDates.spec.js e.g. new Date(2016, 12, 8) -> new Date(2016, 11, 8).

  • Added a test to disabledMonths.spec.js to ensure coverage remains at 100%. This checks that a date's month is disabled when the to year is less than the year of the date in question, thereby covering line 112 of DisabledDate.js.

Hope this makes sense? Let me know if you have any queries.

@mst101 mst101 force-pushed the chore/highlighted-and-disabled-classes branch from 073e856 to 7f2c102 Compare January 11, 2021 19:46
@mst101
Copy link
Contributor Author

mst101 commented Jan 13, 2021

I realised the setters I had written on these classes were pointless, so I removed them. However, if you think we should do some validation on the disabled-dates and highlighted props, let me know and I'll add that.

@MrWook MrWook added the enhancement New feature or request label Jan 13, 2021
@MrWook
Copy link
Collaborator

MrWook commented Jan 13, 2021

On the first glance it looks pretty neat to me, i will take a deeper look tomorrow and test it out 👍
Thanks for the improvements 👍

@mst101
Copy link
Contributor Author

mst101 commented Jan 14, 2021

@MrWook - thanks for your improvements which I have now implemented. Cheers! 😄

@MrWook MrWook merged commit 0a2195f into sumcumo:master Jan 15, 2021
@mst101 mst101 deleted the chore/highlighted-and-disabled-classes branch January 15, 2021 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants