If an event doesn't have a time, why would it not be all day?

Also, passing $date1 twice is incorrect.

CommentFileSizeAuthor
#9 date-1041428-9.patch705 bytestim.plunkett
#7 date-1041428-7.patch879 bytestim.plunkett
#1 date-1041428-1.patch2.53 KBtim.plunkett
date-fix_all_day.patch1.05 KBtim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Priority: Normal » Major
FileSize
date-1041428-1.patch2.53 KB

It also improperly checks for the $increment, and is never passed the $instance at all.

In addition, an event that goes from 00:00 to 23:59 should be considered "all day", regardless of how many days it spans.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented
Status: Needs review » Reviewed & tested by the community

I tested patch #1 with the fullcalendar module and it works great.
I had the same concerns:

1) Improper logic
- 1/2/2011 is not all day but 1/2/2011 00:00:00 is
- an all day event can't span multiple "all day" events

2) Some errors (they absolutly need a fix)
- See original message and #1.

For me this is RTBC.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

Still applies to HEAD.

Ingumsky’s picture

Ingumsky CreditAttribution: Ingumsky commented

Thanks mate. I'll try to apply it.

KarenS’s picture

KarenS CreditAttribution: KarenS commented
Status: Reviewed & tested by the community » Fixed

Yeah, this never got set up right, it was a rough port from D6 that never got finished. Thanks :)

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Version: 7.x-1.x-dev » 6.x-2.x-dev
Status: Closed (fixed) » Needs review
FileSize
date-1041428-7.patch879 bytes

I don't know why I never checked this in D6 originally.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented
Status: Needs review » Reviewed & tested by the community

I can confirm this :)

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
FileSize
date-1041428-9.patch705 bytes

I got my patches mixed up, the intent was to REMOVE the || $date1 != $date2, I got confused because it was removed by #1013662: 'All day' events displaying a time instead of 'All day', but only in D6. The first hunk is the only change to get D6 and D7 on the same page and working correctly.

To simplify, here's another patch. Still RTBC.

Oh, and this patch can be committed using `git am date-1041428-9.patch`, which is what I was referring to in another thread about giving credit.

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
CreditAttribution: klonos commented

...coming from #906232: All day events

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

So I don't lose track, I have to keep using this patch on D6 sites.

fm’s picture

fm CreditAttribution: fm commented

"If an event doesn't have a time, why would it not be all day?"

To answer your question, on one of my sites I display documents with an associated date. The document wasn't published "all day" but at a specific time. In many instances that specific time is unknown, so I neither want to enter an arbitrary and probably wrong time nor do I want the document labeled as "all day." Simply displaying the date is sufficient, but I don't seem to be able to omit this automatically generated "all day" string without tinkering with the page code; and even then that's not ideal since I still have events (which include "all day" events) listed on my site that might be adversely effected by editing the code.

Ideally, this "all day" feature could be turned on and off backstage.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

@fm, I believe http://drupal.org/project/datetweaks could help with your issue.

This patch is simply correcting the logic.

fm’s picture

fm CreditAttribution: fm commented

Thank you for your suggestion. That looks like a very promising solution to my issue.

Ultimately, I believe the automated "all day" feature should not be included with the Date module because it assumes an end use (events) that is narrower than what the scope of the Date module should be. If anything, "all day" should be in an add-on module itself, instead of using an add-on module to strip it out.

arlinsandbulte’s picture

arlinsandbulte CreditAttribution: arlinsandbulte commented

Actually, I think datetweaks won't really change anything regarding 'all day'. It only provides a tweaked input widget to enter date information. The same information is still used, and it is still processed in the same way.

I think what you really want is resolution to this issue:
#874322: To Date & All Day Date Handling - see #4.
I have had some more insight & thoughts after I wrote #4 there, but I have not had a chance to voice them yet.... hopefully soon.

arlinsandbulte’s picture

arlinsandbulte CreditAttribution: arlinsandbulte commented
Status: Reviewed & tested by the community » Fixed

#9 looked right to me. And my testing looked good.
Committed:
http://drupalcode.org/project/date.git/commit/f237dde

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.