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

Render locale specific time stamp #291

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JakeSmarter
Copy link

Currently, the default en-US rendering is often very confusing to users in other locales. This PR renders the time stamp in a locale specific manner.

compact ?
[date[3]] :
[date[1], date[2] + ",", date[3]] :
date).join(" ");
Copy link
Author

Choose a reason for hiding this comment

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

Btw, the current implementation looks a bit overly complex to me or has dead code. Since this will always be true:

new Date(capturedAt).toDateString().split(" ").length > 3

Unless, Date.toDateString() is JavaScript runtime implementation specific, and thus behaves differently on different browsers. But, I am not aware of that.

@JakeSmarter
Copy link
Author

JakeSmarter commented Feb 19, 2019

The time stamp formatting across all Mapillary products, even within products, is quite inconsistent. It should be best to finally make it uniform. Besides, this is very easy to fix. 😉
Alternatively, if you do not want to introduce locale specific time stamps—because you do not want to support multiple locales and just support one international (English) version—then please use ISO 8601 throughout consistently. Then everybody is “on the same page”.

@facebook-github-bot
Copy link

Hi @gitne!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@JakeSmarter
Copy link
Author

I do not want to sound rude but I am still displeased with the current situation with regards to time stamps in Mapillary products and how Mapillary has approached i18n and/or l11n in general, especially since the Facebook takeover. 😞 The internet has always been a place where many cultures and languages meet and communicate, even Mapillary’s parent company has understood this for a long time. If you do not want to support multiple cultures and languages (which is completely acceptable and understandable) then please at least adhere to and be consistent in ISO standards. So, I would be very happy if this PR could find its way into Mapillary products in one way or the other. Thank you!

Base automatically changed from master to main February 24, 2021 09:49
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.

None yet

2 participants