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

misc(treemap): add locale to html tag #12731

Closed
wants to merge 1 commit into from
Closed

misc(treemap): add locale to html tag #12731

wants to merge 1 commit into from

Conversation

coliff
Copy link
Contributor

@coliff coliff commented Jul 1, 2021

Summary

This fixes #12730

The page is missing lang="en" attribute. When loading page in Edge it offered to translate the page from Maltese to English for me. Specifying the lang will tell the browser it's in English already.

Related Issues/PRs

@coliff coliff requested a review from a team as a code owner July 1, 2021 09:52
@coliff coliff requested review from adamraine and removed request for a team July 1, 2021 09:52
@google-cla google-cla bot added the cla: yes label Jul 1, 2021
@adamraine
Copy link
Member

This would fix the problem for english users and the landing page, but we do have some translated strings in the treemap. I'm thinking we have a default value of en-US and programmatically change it when we get the locale:

Basically just stick a

document.documentElement.lang = options.lhr.configSettings.locale

in here

/**
* @param {LH.Treemap.Options} options
*/
init(options) {
TreemapUtil.find('.treemap-placeholder').classList.add('hidden');
TreemapUtil.find('main').classList.remove('hidden');
const i18n = new I18n(options.lhr.configSettings.locale, {
// Set missing renderer strings to default (english) values.

@connorjclark would this work?

Additional ref #12647

@adamraine adamraine changed the title update treemap index.html misc(treemap): add locale to html tag Jul 1, 2021
@adamraine
Copy link
Member

@coliff do you still want this?

@coliff
Copy link
Contributor Author

coliff commented Dec 2, 2021

I'm not 100% certain of exactly where that document.documentElement.lang = options.lhr.configSettings.locale goes in the main.js so if someone else could kindly take care of that then that'd be awesome.

@adamraine
Copy link
Member

Closing in favor of #13454. I'll add you as a co-author.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lighthouse Treemap webpage is missing html lang attribute
4 participants