Replace jQuery.timeago #3123
Replace jQuery.timeago #3123
Conversation
|
I had assumed the JS locale is provided by the browser on the client-side? |
|
Use How many locales do you want to supported, may be I can help？ |
|
@mperham I could use the navigator.language API but I figured that it would be better to align fully with Rails/web app. |
|
Good call. Being consistent is more important than being right. |
| @@ -29,5 +28,10 @@ | |||
| </div> | |||
| <%= erb :_footer %> | |||
| <%= erb :_poll_js %> | |||
| <script type="text/javascript"> | |||
There was a problem hiding this comment.
Please don't use inline scripts, this'd make enforcing Content Security Policies a lot more difficult.
There was a problem hiding this comment.
Do you have any suggestions as to how I can pass the value to JavaScript without using an inline script?
There was a problem hiding this comment.
Is there a reason why adding this exact code to the application.js (or equivalent) doesn't work?
There was a problem hiding this comment.
We pass a variable from Rails to the JavaScript app to load the correct locale (<%= locale %>).
I don't see how we can have the JS app use the same locale as Rails without passing this through.
Previously this was done by loading a specific file by name, but the idea of this PR is to reduce the overhead of multiple JS files.
There was a problem hiding this comment.
Also, it's worth noting that _poll_js already uses inline JavaScript.
There was a problem hiding this comment.
Could you just add a data attribute to the body element and get the locale from that?
e.g. <body class="admin" data-locale="<%= locale %>"> and then
var locale = $('body').data('locale');
window.addEventListener('load', function() {
timeago().setLocale(locale);
});There was a problem hiding this comment.
Perfect - I'll do that, thanks!
There was a problem hiding this comment.
If someone wants to send a PR changing the poll_js logic in the same manner, I'd love it.
There was a problem hiding this comment.
Can I open an issue to make sure it doesn't get forgotten in the mean time? :)
There was a problem hiding this comment.
Absolutely, that's what it's there for!
This library is ~2KB, has no dependencies and includes several built-in locales. It also updates the timestamp on the page in realtime.
The localepassed by Rails/browser "Accept-Language" header needs to be manipulated before being used by timeago.js
|
OK, this PR is ready for review now. Sorry it took so long! |
|
I don't like the "live update" at all. Can we turn it off? It will lead to bug reports: "Next retry: 10 seconds ago". |
|
Absolutely. PR incoming On Sep 20, 2016 14:19, "Mike Perham" notifications@github.com wrote:
|
|
Just did it. |
Replace the heavy jQuery.timeago plugin with a smaller library.
update with new translations (PR open on source library)timeago.jsbuild with all locales included - waiting on this issueThe list of included locales can be found here.