The current date range separator for fields that collect both a start and end date is a hyphen. This can be a bit confusing and unclear in some situations (especially in cases where times are being displayed, since a hyphen is also used to separate the date from the time).

Jeff Noyes suggests changing it to the word 'to', which seems pretty reasonable to me since that's a pretty common way to refer to date ranges.

The attached patch makes that change in the default theme function. Any reason not to do that?

CommentFileSizeAuthor
date-separator-to.patch1.07 KBDavid_Rothstein

Comments

karens’s picture

karens commented
Status: Needs review » Fixed

OK by me. I see you also removed the css class for the separator. Don't know if any will care about that or not, but I guess we'll see. It's a new release cycle so it should be OK.

karens’s picture

karens commented

Arg, this broke a bunch of tests that were using AssertText(). Fixing that now.

David_Rothstein’s picture

David_Rothstein commented

Oops, sorry about that. I'm too used to working in the core queue where the testbot automatically runs tests for me...

omercioglu’s picture

omercioglu commented
Status: Fixed » Active

Please put "date-display-separator" class back, perhaps like below:

  return t('!start-date !separator !end-date', array(
    '!start-date' => '<span class="date-display-start"' . drupal_attributes($attributes1) . '>' . $date1 . '</span>',
    '!separator' => '<span class="date-display-separator">'.t('to').'</span>',
    '!end-date' => '<span class="date-display-end"' . drupal_attributes($attributes2) . '>' . $date2 . $timezone. '</span>',
  ));
David_Rothstein’s picture

David_Rothstein commented

I think that would cause problems for translators. (For example, in some languages, "to" isn't even its own word.)

With all the other classes in this area of the code, I think most simple theming needs can already be met, can't they? (For example, theme the parent item the way you want "to" to appear and then theme the other parts to undo that.)

For more complex needs, you can always override theme_date_display_range() entirely and add whatever classes you want. One thing that might be worth doing here (to help make that simpler) is to move all the RDF-related code out of that theme function and into a preprocess function. It probably belongs in the preprocess layer anyway, and that way the theme function itself would be a lot simpler (just HTML) and therefore cleaner to override.

David_Rothstein’s picture

David_Rothstein commented

I just wrote a patch for that here: #1253248: Move RDF-related code to theme preprocess functions, to make the theme functions easier to override

I think this issue should probably be closed, since if there are any further followups they could be done elsewhere.

karens’s picture

karens commented

I can see the potential benefit of a separator class, and we've had one in the past. The way it was proposed in #4 would probably be a translation problem, but I think it could be done like this and not be too difficult to translate (since we keep the context):

 return t('!start-date <span class="date-display-separator>to</span> !end-date', array(
    '!start-date' => '<span class="date-display-start"' . drupal_attributes($attributes1) . '>' . $date1 . '</span>',
    '!end-date' => '<span class="date-display-end"' . drupal_attributes($attributes2) . '>' . $date2 . $timezone. '</span>',
David_Rothstein’s picture

David_Rothstein commented

Yeah, I guess that would work. It's a bit unfortunate to have that much HTML in a translatable string, but I agree it wouldn't break anything.

karens’s picture

karens commented
Status: Active » Fixed

Well, let's keep it clean and leave it out. Someone who wants the wrapper can override the theme and add it. If you're doing anything with the separator you are probably already doing some custom theming, so that shouldn't be an enormous burden.

Status: Fixed » Closed (fixed)

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

cristian100’s picture

cristian100
Primary language Spanish
Location Tepic Nayarit
commented

Ohh man, In my case I really need it to be a hyphen, why don't have a feature to customize the character here?? at timefield module I really love that I can type the separator of times ranges.

HeathN’s picture

HeathN commented

We need to make this more dynamic. Purposed change here 'https://drupal.org/node/2237843'.