BrightspaceUILabs / file-uploader

Polymer-based web component for D2L file uploader
Apache License 2.0
4 stars 6 forks source link

Add a localize behavior with translations #50

Closed alogan519 closed 7 years ago

alogan519 commented 7 years ago

Add a localize behavior with translations instead of trying to load resources through polymer resolve url

dlockhart commented 7 years ago

CI appears to be failing from a polymer lint error.

dlockhart commented 7 years ago

More linting errors in CI. You should be able to just run npm test locally to catch them all before pushing. :)

dlockhart commented 7 years ago

Just be aware that @ianmclean2011 is working in the translation file with another PR, so you guys are going to end up with conflicts.

alogan519 commented 7 years ago

Ok ya looks like Ian also removed the IE 11 test for Windows 10 which is failing for me.

dlockhart commented 7 years ago

Yeah I tried all morning to figure out what's broken but I think it's something core in polymer-cli or the polyfill. I removed IE11 temporarily until it gets fixed.

alogan519 commented 7 years ago

I pulled in the latest from Ian's change.

ianmclean2011 commented 7 years ago

I was going to say that you need to change the Japanese tag to be "ja" instead of "ja-jp", which was a problem that I fixed with my recent changes. However, I see that they changed the lang string in the lms to be ja-jp, instead of just ja (looks like they did that for 10.7.3 ... http://search.dev.d2l/source/xref/Lms/core/lp/_dbchange/10.7.3.0/main/pre/update_ja_locale_code.sql).

That looks correct, and the frau-locale-provider might need to be changed, and subsequently different FRA lang files may need to be changed. Thoughts @dlockhart?

dlockhart commented 7 years ago

Yikes... I didn't see that they'd done that.

So I think most existing FRAs will be OK, since frau-locale-provider it takes what the LMS provides and will first look for the region (ja-JP) in its mappings, and if it doesn't find that it takes the beginning part (ja) and returns that.

Which should work for existing FRAs that just have the ja entry in their language files. But going forward, if they start putting ja-JP in there, it's going to fail. And yeah, web components like this that don't have as much logic are also going to start failing.

Thanks for raising this. I'm going to take it away to my team and we'll likely come up with some stories/defects to get things fixed.

alogan519 commented 7 years ago

So I started working on adding in the "extra" mappings as well, so like adding in ja as well as ar and just copying over the translations from ja-jp and ar-sa. That seems to be what we are getting back for the translations currently.

dlockhart commented 7 years ago

Cool. Yeah I'd say eventually we just want to have entries for "ja", "en", "fr", etc. and then only put overrides into "fr-CA", etc. But currently that would involve you manually combining collections.

Doing this automatically I think would be a good additional feature for the locale-helper-whatever utility I want to build -- maybe for innovation sprint.