Skip to content

Conversation

Rican7
Copy link
Contributor

@Rican7 Rican7 commented Feb 28, 2015

This small PR provides a nice little feature on top of a somewhat bug-fix.

Previously, the back-end was passing date values to the react front-end without a timezone. And while this project delivers on the best-practice of using UTC timezones for the back-end, that doesn't play so well once the data is passed to a front-end. See, browsers date handling (MomentJS's included) assumes that all time-zone-less date/time values are in the same time-zone as the client browser. Unfortunately that meant that the times in the timeline were always off by 5 hours for me (I'm in EST). This PR fixes that issue by always passing an ISO-8601 (includes timezone) date string to the front-end to handle.

While fixing that issue was ideal, this PR also aims to subtly improve the user experience of "PHP RFC Watch" by using the power of MomentJS's relative time handling. Now, when displaying times in the event timeline, we'll display a relative time in a more semantic HTML5 time tag (falls back to a span for older browsers). To give more clarity and to uphold an expected UI pattern, the time tag also uses the title attribute to provide a fully detailed time string when hovering over the element. Finally, we use the time tag's semantic data format to provide a client-parsable date format so that javascript extensions and added browser behaviors could be powered by the raw data. :)

Finally, I added a very small margin to the clock icon that prefixes the actual date string in the UI, just to relieve a minor cluttered appearance that I found when testing the date strings.

I'm really loving this project! Thanks for the awesome work. 😃

values in a standardized date-format which includes a timezone, so that
the client (and `moment.js`) can correctly parse the date in local time.
format for display (thanks Moment.js!), while providing a more detailed
format on hover.
Also using the more semantic `<time>` tag for good measure. ;)
beberlei added a commit that referenced this pull request Feb 28, 2015
@beberlei beberlei merged commit e914c51 into beberlei:master Feb 28, 2015
@beberlei
Copy link
Owner

@Rican7 merged and deployed, but i think the relative dates are wrong. It lists davey as 8 hours ago for me, but I think he has voted for a much longer time now.

@Rican7
Copy link
Contributor Author

Rican7 commented Feb 28, 2015

Woot! Thanks for the merge. :)

And regarding the accuracies, it looks right to me:
2-28-2015-2-09-57-am-7f5e

The only thing that I did notice is that an incoming vote came in (jgmdev's) as I was toying with the relative dates, and it said 10 minutes from now. I checked the RFC wiki source and sure enough the date was in the future by several minutes. So it looks like the only real problem is that the PHP RFC wiki server could use some NTP. 😛

@Rican7 Rican7 deleted the feature/client-local-time branch February 28, 2015 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants