Skip to content

Expose ComputePlatformResolvedLocale #58482

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.

Sign up for GitHub

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

Jump to bottom
Merged
GaryQian merged 11 commits into from
Jul 1, 2020
Merged

Expose ComputePlatformResolvedLocale #58482

GaryQian merged 11 commits into from
Jul 1, 2020

Conversation

@GaryQian
Copy link
Contributor

@GaryQian GaryQian commented Jun 2, 2020 •
edited
Loading

Exposes ComputePlatformResolvedLocale for #47087

Relevant code added in flutter/engine#18645 and flutter/engine#19266

@fluttergithubbot fluttergithubbot added framework flutter/packages/flutter repository. See also f: labels. work in progress; do not review labels Jun 2, 2020
@GaryQian GaryQian changed the title Initial implementation Embedder localization resolution Jun 2, 2020
@GaryQian GaryQian mentioned this pull request Jun 9, 2020
Merged
@GaryQian
Copy link
Contributor Author

GaryQian commented Jun 16, 2020

Will hold off on this until the TODOs introduced in flutter/engine#18645 are resolved to ensure the feature produces the correct values.

@GaryQian
Copy link
Contributor Author

GaryQian commented Jun 30, 2020

This method is a direct call to dart:ui's Window.computePlatformResolvedLocale and is tested at the engine level of each platform.

@GaryQian GaryQian marked this pull request as ready for review June 30, 2020 02:36
@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Jun 30, 2020

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@GaryQian GaryQian added a: internationalization Supporting other languages or locales. (aka i18n) and removed work in progress; do not review labels Jun 30, 2020
@GaryQian GaryQian requested a review from HansMuller June 30, 2020 06:15
@GaryQian GaryQian changed the title Embedder localization resolution Expose ComputePlatformResolvedLocale Jun 30, 2020
@GaryQian GaryQian requested a review from justinmc June 30, 2020 10:50
@GaryQian GaryQian mentioned this pull request Jun 30, 2020
Closed
justinmc
justinmc approved these changes Jun 30, 2020
View reviewed changes
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

CC @shihaohong in case this is relevant.

packages/flutter/lib/src/widgets/binding.dart Outdated
Comment on lines 978 to 979
Copy link
Contributor

@justinmc justinmc Jun 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// it is recommended to cache the results of this method if the value if it
/// is used multiple times.
/// it is recommended to cache the results of this method if the value is
/// used multiple times.

@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Jul 1, 2020

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite analyze-linux has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite deploy_gallery-linux has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite deploy_gallery-macos has failed. Please fix the issues identified (or deflake) before re-applying this label.

@GaryQian GaryQian merged commit 0e735e4 into flutter:master Jul 1, 2020
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@GaryQian @mingwandroid
Expose ComputePlatformResolvedLocale (flutter#58482)
ce0f137
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@justinmc justinmc justinmc approved these changes

@HansMuller HansMuller Awaiting requested review from HansMuller

Assignees

No one assigned

Labels

a: internationalization Supporting other languages or locales. (aka i18n) customer: money (g3) framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@GaryQian @fluttergithubbot @justinmc @googlebot