-
-
Notifications
You must be signed in to change notification settings - Fork 30
fix current date and add specific set date #22
base: master
Are you sure you want to change the base?
Conversation
hey @luciluci somehow I wasn't notified about the pull request! Sorry about that. Also, thank you so much for submitting a pull request 😃 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@luciluci thank you for your contribution! ❤️
I added a couple of comments regarding the changes, but there's some other things that will be mentioned below.
Adding an Option to Switch Dates
Having the current date selected, still, even after switching the month is a feature and not a bug. The reason behind that is that sometimes a person would like to switch months, but keep the organizer for that day selected (so that the events are visible).
My point is, I'd like you to add an "option"/"setting" similarly to what you did to the custom_date
.
By doing so, people can choose to switch the month and have the selected day either relative to the month selected, or absolute to the whole calendar.
The option I have in mind is:
- key:
day_selection
- values:
relative
,absolute
Simplifying the Example
I'm really glad you provided an example with the changes, and I really appreciate you doing so.
But I personally got lost in the example, and it felt like it's too big to just showcase how you can set the initial date.
Can you please, instead, use the normal_calendar.html
example as a base, and then add the custom_date
to it?
Also, can you also add a similar example, also based on the normal_calendar.html
for the day_selection
option?
Adding Necessary Documentation of the Changes
Please update the README based on the changes you did. It would help keep users well informed of the new things 🙂
indicator: true, | ||
indicator_type: 0, // indicator type, can be 0 (not numeric) | 1 (numeric) | ||
indicator_pos: "top", // indicator position, can be top | bottom | ||
custom_date: test_day |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please change the custom_date
to something like initial_date
or pre_selected_date
, those are a bit more informative than custom_date
👍🏼
if (options.custom_date != undefined) { | ||
this.date = options.custom_date; | ||
} | ||
else{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please tweak this line so it looks like the following example?
if () {
// something
} else {
// something else
}
fixed the bug: after selecting a specific date, when changing months, the selected date remained 'checked'
added possibility to create calendar with specific date different than today