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

Referential issue with default values for defaults.position.start and defaults.position.end #112

Closed
OOPMan opened this issue Sep 27, 2013 · 7 comments
Labels

Comments

@OOPMan
Copy link

OOPMan commented Sep 27, 2013

There is a peculiar referential issue relating to the Date instances assigned to the defaults.position object that only becomes apparent when attempting to use multiple Calendars on a single page, as I am doing for a project which requires inter-linked Calendars rather than the drill-down type support by bootstrap-calendar.

See below for a simple example:

// Create a Month-view Calendar initialised to Oct 2013
monthCalendar = $("#monthCalendar").calendar({
  events_url: "url",
  view: "month",
  day: "2013-10-01"
});

// Create a Week-view
weekCalendar = $("#weekCalendar").calendar({
  events_url: "url",
  view: "week",
  day: "2013-09-23"
});

// Now, if we attempt to use monthCalendar.navigate it will behave incorrectly
monthCalendar.navigate("next"); // This will not seem to load the next month
monthCalendar.navigate("next"); // This will load the next month!

I dug into things using the Chrome debugger and noted that during the this._init_position() call during the construction of the weekCalendar object that the Date instance pointed to by this.position.start was the same as the Date instance pointed to by the defaults.position.start.

The result of this was that the .start and .end values for monthCalendar get over-written during the constructon of the weekCalendar and result in the monthCalendar having .start and .end values that do not match what it was configured with initially.

This in turn causes the navigation feature to behave erratically.

I was able to circumvent this issue as follows:

// Create a Month-view Calendar initialised to Oct 2013
monthCalendar = $("#monthCalendar").calendar({
  events_url: "url",
  view: "month",
  day: "2013-10-01",
  position: {
    start: new Date(),
    end: new Date()
  }
});

// Create a Week-view
weekCalendar = $("#weekCalendar").calendar({
  events_url: "url",
  view: "week",
  day: "2013-09-23",
  position: {
    start: new Date(),
    end: new Date()
  }
});

// Now, if we attempt to use monthCalendar.navigate it will behave correctly
monthCalendar.navigate("next"); // This will load the next month!

The underlying issue here is that jQuery.extend does not really support a true deep-copy it seems and is unable to clone instances of objects like Date when performing a deep-copy.

I would recommend that the values for defaults.position.start and defaults.position.end be set to null and then initialised to a new Date() instances following the call to jQuery.extend in the Calendar constructor function.

@Serhioromano
Copy link
Owner

Could you submit PR which fixes this?

@OOPMan
Copy link
Author

OOPMan commented Sep 29, 2013

Sure, I'll try and do so this week :-)

@Serhioromano
Copy link
Owner

Excellent!

@mlocati
Copy link
Contributor

mlocati commented Oct 1, 2013

To highlight this problem see this sample script: http://jsfiddle.net/eUYDK/

@mlocati
Copy link
Contributor

mlocati commented Oct 1, 2013

A solution could be to remove position from the defaults variable and update the constructor by using this line:

this.options = $.extend(true, {position: {start: new Date(), end: new Date()}}, defaults, params);

@mlocati
Copy link
Contributor

mlocati commented Oct 1, 2013

If it's ok I'd fix this bug

@OOPMan
Copy link
Author

OOPMan commented Oct 1, 2013

Nice fix, thanks guys :-)

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

No branches or pull requests

3 participants