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

Make the language switching code more robust #234

Merged
merged 2 commits into from
Aug 16, 2020
Merged

Conversation

xfq
Copy link
Member

@xfq xfq commented Aug 15, 2020

Since language switching in ED has been unable to work for a few days now (see #231), I made a pull request to fix this issue first. I suggest that we merge it, and we can continue to improve the code in the future.

@xfq xfq requested review from himorin and r12a August 15, 2020 09:58
@himorin
Copy link
Contributor

himorin commented Aug 16, 2020

we may not have any additional code for check or debug in the future, but how about to add one function like below for ease of maintenance and modification.

// as replaceHeaders('editors', translations[lang].editors);
function replaceHeaders (target, translated) {
    if (document.getElementById(target)) {
        document.getElementById(target).textContent = translated;
    }
}

@himorin
Copy link
Contributor

himorin commented Aug 16, 2020

Ah, one point. Do we want to add a line for console log output, for telling us about error?

@xfq
Copy link
Member Author

xfq commented Aug 16, 2020

It is indeed a good suggestion. I just refactored the code.

I used console.log instead of console.error, because sometimes missing headers may not be considered errors (like previousversion).

@xfq
Copy link
Member Author

xfq commented Aug 16, 2020

Thanks for the review @himorin. I'll go ahead and merge this.

@r12a feel free to comment on #231 or submit a new issue/PR if you have any question/concern/suggestion about this.

@xfq xfq merged commit 33d9f6a into gh-pages Aug 16, 2020
@xfq xfq deleted the xfq/switch-lang-fix branch August 16, 2020 14:20
@r12a
Copy link
Contributor

r12a commented Aug 17, 2020

@r12a feel free to comment on #231 or submit a new issue/PR if you have any question/concern/suggestion about this.

From a quick scan i don't notice anything problematic. (However, i would have preferred to keep the layout consistent, ie. no ; at line end, and } in line with block end (Python-like layout). That's only a minor thing though.)

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.

3 participants