Skip to content
This repository was archived by the owner on Mar 4, 2021. It is now read-only.

fix current date and add specific set date #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

luciluci
Copy link

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

@nizarmah nizarmah self-requested a review December 17, 2020 19:23
@nizarmah
Copy link
Owner

hey @luciluci somehow I wasn't notified about the pull request! Sorry about that.
I'll give this a review in the next couple of days, I'm really sorry.

Also, thank you so much for submitting a pull request 😃

Copy link
Owner

@nizarmah nizarmah left a 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
Copy link
Owner

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{
Copy link
Owner

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
}

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

Successfully merging this pull request may close these issues.

2 participants