Skip to content

Commit

Permalink
Make sure we are consistent with format in DS
Browse files Browse the repository at this point in the history
@PascalPiche came across this bug while displaying the weekday name: The DS formats the weekday with 'N', which uses ISO-8601 (first date of the week is then Monday as in the US, not Sunday).

There were multiple places where a fix could be applied, but the best solutions we found was to apply the fix in the data-source for two reasons:

1. Field ouput won't change, so less risk to break things
2. The xsl template is agnostic to this change since it was doing string matches
  • Loading branch information
nitriques committed Nov 21, 2014
1 parent 1f18eec commit 420841e
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion data-sources/data.datetime.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public function grab(array &$param_pool=NULL) {

// Weekdays
$storage['weekdays'] = array();
$date->modify('last Sunday');
$date->modify('last Monday');
for($i = 1; $i <= 7; $i++) {
$storage['weekdays'][] = $date->getTimestamp();
$date->modify('+1 day');
Expand Down

6 comments on commit 420841e

@nilshoerrmann
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this actually solving?

@nitriques
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The datetime xsl template was rendering one day off. Like today was Thrusday nov 22, which is wrong.

@nilshoerrmann
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. I'll have to think about the sideeffects of this change.

@nitriques
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please do! The only one I've found was that if you did apply-templates on the weekdays (to create some kind of calendar), then the order would change. If not, I could not find another side effect.

@nilshoerrmann
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the change is fine for now, but we should implement #130 with a proper setting.

@nitriques
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

Please sign in to comment.