Repository navigation
Implement local, locales, and onLocaleChanged for the web - #18137
Conversation
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
1 similar comment
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
@slightfoot Can you please OK including your commits in this PR? #18137 (comment) has the details. |
|
Yes. Your welcome to my commits. |
Thanks! Looks like the googlebot wants you to follow this small instruction: We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only |
|
@googlebot I consent |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
1 similar comment
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
| _onLocaleChangedZone = Zone.current; | ||
| } | ||
|
|
||
| /// The locale used when we fails to get the list from the browser. |
| // To make it easier for the app code, let's not leave the locales list | ||
| // empty. This way there's fewer corner cases for apps to handle. | ||
| if (locales.isEmpty) { | ||
| locales.add(_defaultLocale); | ||
| } |
There was a problem hiding this comment.
Suggestion: return early before the for loop.
if (!js_util.hasProperty(html.window.navigator, 'languages')) {
return const [_defaultLocale];
}
if (html.window.navigator.languages.isEmpty) {
return const [_defaultLocale];
}
for (final String language in html.window.navigator.languages) {
// ...
}
return locales;This is just a personal preference of mine. It makes reading the code easier when edge cases are handled at the beginning.
There was a problem hiding this comment.
Done. I actually prefer that too, but missed it this time.
slightfoot
left a comment
slightfoot
left a comment
There was a problem hiding this comment.
I have not run the code changes yet. It looks good, however there's quite a few formatting based line changes in the change set. I'll see if I can find some time today or tomorrow to test it out.
|
@slightfoot LMK whether this is working for you. I tested by changing the browser language to "ru" and running the new Flutter Gallery (see screenshot in the PR description). |
…ter-team-archive#18137) * implement `locale`, `locales`, and `onLocaleChanged` in the web version of `Window. Co-authored-by: Simon Lightfoot <[email protected]>
4 participants
Fixes flutter/flutter#54669
Fixes flutter/flutter#48886
Here's the new gallery after I changed my Chrome default language to Russian: