kpmg-agile / ca-pqvp

KPMG submission for CA PQVP
https://www.calproc.website
7 stars 3 forks source link

No longer using the i18next language property when determining the cu… #242

Closed crayl-kpmg closed 7 years ago

crayl-kpmg commented 7 years ago

…rrent language setting in the language selector during init. It seems to not be reliable. Instead now using the value in local storage that represents the same data.

Resolves #235

robertlevy commented 7 years ago

hmmm... does this work if the localstorage item isn't there? the problem with the i18n event is that it happens asynchronously

oh crap... i bet that's actually the root cause here... the async stuff i18next is doing is probably outside the angular2 zone. maybe we just need to manually kick the zone in the i18next event handler?

robertlevy commented 7 years ago

ngzone doesnt seem to be the issue. asynchronous loading is though. i tweaked this branch with a different approach that plays nicer with i18next

crayl-kpmg commented 7 years ago

I realize this is closed, but just so that you aren't left hanging I'll speak to some of this.

The issue as I understood it was not the application of the localization, it was really just the dropdown not properly rendering the current language when it was initialized. This was because the i18next.language value was often null, even when it was actually set. That is why I switched to the localStorage data. Your option to OR the default config serves the same purpose.

Yes, the setup should have worked if the value was not set. It would detect that state and set it.

As for the zone, I am not sure that is really something we need to concern ourselves with. I don't think anything else will be changing the language beyond the init process for the app and this single control. But the issue wasn't just during startup on the initial localization setup being async, the dropdown would fail often times after the app had been running.

robertlevy commented 7 years ago

Yeah this all got a lot more complicated than it probably should be. Need to take a fresh look at this for the sake of future projects. In prior versions of the accelerator we delayed the loading of the UI framework until language was determined. Might need to go back to that.

On Mar 1, 2017, at 10:46 AM, crayl-kpmg notifications@github.com<mailto:notifications@github.com> wrote:

I realize this is closed, but just so that you aren't left hanging I'll speak to some of this.

The issue as I understood it was not the application of the localization, it was really just the dropdown not properly rendering the current language when it was initialized. This was because the i18next.language value was often null, even when it was actually set. That is why I switched to the localStorage data. Your option to OR the default config serves the same purpose.

Yes, the setup should have worked if the value was not set. It would detect that state and set it.

As for the zone, I am not sure that is really something we need to concern ourselves with. I don't think anything else will be changing the language beyond the init process for the app and this single control. But the issue wasn't just during startup on the initial localization setup being async, the dropdown would fail often times after the app had been running.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHubhttps://github.com/kpmg-agile/ca-pqvp/pull/242#issuecomment-283377007, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AB1xQuvGqRlJCZMRPMPDa4qjjpoPclrVks5rhZKPgaJpZM4MPIrX.