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

time_tag helper doesn't use Y-m-d format for date #9333

Closed
NARKOZ opened this issue Feb 20, 2013 · 7 comments
Closed

time_tag helper doesn't use Y-m-d format for date #9333

NARKOZ opened this issue Feb 20, 2013 · 7 comments

Comments

@NARKOZ
Copy link
Contributor

NARKOZ commented Feb 20, 2013

helper.time_tag Item.first.created_at.to_date
# => "<time datetime=\"2013-02-14T00:00:00+00:00\">February 14, 2013</time>"

I expected:

<time datetime="2013-02-14">February 14, 2013</time>

as in docs.

@senny
Copy link
Member

senny commented Feb 20, 2013

I'll take a look at it and report back.

@senny
Copy link
Member

senny commented Feb 20, 2013

I can reproduce the described report. The question now is, is the code wrong or the documentation. After a little investigation I found:

  • There was a bug report on ruby core because Date#rfc339 did not include the time.
  • the HTML standard allows both, times and dates

The bug reports could mean that the documentation once reflected the actual behavior (Date#rfc339 is used in the assert). We still have two possibilities to fix the problem:

  1. Patch the code (change to date format for dates)
  2. Patch the docs (keep datetime format for dates)

I'm in favor of 1.) because I don't see why we should include the needless "00:00:00+00:00".

@carlosantoniodasilva @pixeltrix let me know which route to take and I'll submit a patch.

@pixeltrix
Copy link
Contributor

I'd patch it to use to_s(:db) instead of rfc3339. I'd also add a documentation example on how to do a custom datetime attribute as well since it's not readily apparent how you'd do some of the other formats allowed like , e.g:

<%= time_tag "Week #{date.cweek}", :datetime => date.strftime('%G-W%V') %>
<time datetime="2013-W08">Week 8</time>

@senny
Copy link
Member

senny commented Feb 20, 2013

@NARKOZ I see you created a PR, do you want to update it to what @pixeltrix said?

@pixeltrix
Copy link
Contributor

@senny @NARKOZ iso8601 is fine as well - just add the doc change also

@NARKOZ
Copy link
Contributor Author

NARKOZ commented Feb 20, 2013

I've updated docs

@senny
Copy link
Member

senny commented Feb 22, 2013

I'm closing this since #9334 was merged.

@senny senny closed this as completed Feb 22, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants