search

raucao / webmarks

remoteStorage-enabled bookmarking app
https://webmarks.5apps.com
Other
76 stars 11 forks source link

Localize app #56

Closed ebrahim-elgaml closed 6 years ago

ebrahim-elgaml commented 7 years ago

Connected to #12

Localize all the pages with ember-i18n Import moment locale add translation for English and German

Note: Check German translation as it is all based on google translate

raucao commented 7 years ago

@ebrahim-elgaml I added one suggestion about improving the code, but it all LGTM in general. Good job!

ebrahim-elgaml commented 7 years ago

@skddc Please check again now

raucao commented 7 years ago

@ebrahim-elgaml Did you actually try this out in multiple browsers? The navigator APIs are inconsistent between browsers oftentimes, and in this case, the language one seems to not work in Firefox as it is now:

TypeError: can't define array index property past the end of an array with non-writable length

From this line:

userLanguages.push(navigator.language, navigator.userLanguage, 'en');

The error prevents the whole app from loading, so I get a blank screen in FF currently.

raucao commented 7 years ago

Ping @ebrahim-elgaml

ebrahim-elgaml commented 7 years ago

@skddc Sorry I have not seen that comment before. I have tested it only on chrome. I will work on that issue

raucao commented 7 years ago

Great, thanks!

ebrahim-elgaml commented 7 years ago

@skddc please check now

raucao commented 7 years ago

It'd be nice if your commit message (or at least the GitHub comment) would contain an explanation of how your change fixes the issue (i.e. what the issue actually was and why it's fixed now).

Otherwise I have to assume things instead of knowing for sure what the intent of the code is.

Thanks!

ebrahim-elgaml commented 7 years ago

@skddc I have pushed again. Sorry for the commit description Please check again now

raucao commented 7 years ago

Thanks!