continuouscalendar / jquery-continuous-calendar

Date picker and range selector with scrollable months instead of paged (the only right way to do it)
http://continuouscalendar.github.io/jquery-continuous-calendar/
85 stars 35 forks source link

mousedown event is not working for current day when selecting new range #35

Closed eeroan closed 12 years ago

KevinTriplett commented 12 years ago

@reaktor and @eeroan: May I help fix this issue? The problem is the div wrapper for the current day in the table cell: it becomes the event.target and throws everything else off since no events are attached to it and it does not have class .date or attributes like "date-cell-index" ...

The fix can be clean or messy. Since the div creates a special case, I propose removing the div. As pretty as the circle is around the current date, this is the cleanest solution. Which means putting an ugly square red border around the current date table cell. :b

The only other solutions I can think of are messy. Any thoughts or advice? How important is the circle border around current date?

eeroan commented 12 years ago

On 21.3.2012, at 1.52, Kevin Triplett wrote:

@reaktor and @eeroan: I would like to fix this issue. The problem is the div wrapper for the current day in the table cell: it becomes the event.target and throws everything else off since no events are attached to it and it does not have class .date or attributes like "date-cell-index" ...

Great, if you have time to fix it. I'm looking forward.

The fix can be clean or messy. Since the div creates a special case, I propose removing the div. As pretty as the circle is around the current date, this is the cleanest solution. Which means putting an ugly square red border around the current date table cell. :b

Perhaps the inner div could delegate event to it's parent element, like this:

circle.mousedown(function(e) {circle.parent.trigger(mousedown, e)})

UX is more important than how the code looks like, even that is the second most important thing.

The only other solutions I can think of are messy. Any thoughts or advice? How important is the circle border around current date?

It solves two problems

1) the red border surrounds the current day even when it is next to left or right end of the table (this was broken before)

2) the current day is easier to spot and works also for colour blind people :)

KevinTriplett commented 12 years ago

Ah, that all makes sense (re UX and history, I knew there was a reason for the div). Okay, I'll tackle this one, thanks for the tips!

KevinTriplett commented 12 years ago

Sorry, I inadvertently opened a new issue (I'm not hip to github's pull requests yet). I came up with a simple solution:

      function getElemDate(elem) { return dateCellDates[$(elem).closest('[date-cell-index]').attr('date-cell-index')] }

But as always the tests were trickier. Trying to create a calendar with the current date in a known location was interesting. But critical to this was modifying the test utility that returns the target for the mouse event, it has to return the div. Not the best solution but works for this case. Hopefully this particular change to the utility is okay with everyone.

function mouseEventOnDay(eventType, date, options) {
  var elem = cal().find('.date div').withText(date).length === 0 ?
    cal().find('.date').withText(date) :
    cal().find('.date div').withText(date);
  mouseEvent(eventType, elem, options);
}