search

w3c / jlreq

Text Layout Requirements for Japanese
https://w3c.github.io/jlreq/
Other
101 stars 17 forks source link

Make the language switching code more robust #234

Closed xfq closed 4 years ago

xfq commented 4 years ago

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.

himorin commented 4 years ago

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 commented 4 years ago

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

xfq commented 4 years ago

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 commented 4 years ago

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

@r12a feel free to comment on https://github.com/w3c/jlreq/issues/231 or submit a new issue/PR if you have any question/concern/suggestion about this.

r12a commented 4 years ago

@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.)