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

Add date to listings #31

Merged
merged 2 commits into from Apr 22, 2016

Conversation

Projects
None yet
3 participants
@reynir
Contributor

reynir commented Apr 22, 2016

Thanks for the feedback in #28 ! It was quite simple.

The output will look like

Written by Foobar Baz (22.04.2016)

I put the timestamp in a with the date class. I'm not sure if you agree with this. HTML and CSS is not my forte :-)

@dbuenzli

This comment has been minimized.

dbuenzli commented Apr 22, 2016

Written by Foobar Baz (22.04.2016)

I'd suggest using big-endian dates i.e. 2016-04-22, those are universally recognized as unambiguous which is not the case of little-endian ones where month and day numbers can misinterpreted depending on your locale.

I put the timestamp in a with the date class. I'm not sure if you agree with this. HTML and CSS is not my forte :-)

I'd rather use a time element and no class.

@Engil

This comment has been minimized.

Owner

Engil commented Apr 22, 2016

@reynir: Thank you! :)

I'd suggest using big-endian dates i.e. 2016-04-22, those are universally recognized as unambiguous which is not the case of little-endian ones where month and day numbers can misinterpreted depending on your locale.

I don't have anything to say against modifying how the date is displayed, big-endian dates looks indeed rather unambiguous. (changing them is as simple as editing the ptime_to_pretty_date function)

I'd rather use a time element and no class.

Didn't knew about the time element, again I have nothing against using it, if you want to change it @reynir go ahead. :)

@reynir reynir referenced this pull request Apr 22, 2016

Merged

Big-endian dates #32

@reynir

This comment has been minimized.

Contributor

reynir commented Apr 22, 2016

I changed the output into a element. The timestamp is relatively big next to the author field. Maybe the CSS should be changed(?)

@Engil

This comment has been minimized.

Owner

Engil commented Apr 22, 2016

Will fix it after merging, everything looks good to me otherwise. So, merging. :)
Thank you!

@Engil Engil merged commit 9bb46a9 into Engil:master Apr 22, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Engil

This comment has been minimized.

Owner

Engil commented Apr 22, 2016

And it's done, thank you again. :)

@reynir

This comment has been minimized.

Contributor

reynir commented Apr 22, 2016

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment