Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

For #15796 - Use the device locale for the system locale subtitle #15975

Merged
merged 1 commit into from Oct 17, 2020
Merged

For #15796 - Use the device locale for the system locale subtitle #15975

merged 1 commit into from Oct 17, 2020

Conversation

gabrielluong
Copy link
Member

Fixes #15796. Follow up from #15966 (review)

Pull Request checklist

  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

To download an APK when reviewing a PR:

  1. click on Show All Checks,
  2. click Details next to "Taskcluster (pull_request)" after it appears and then finishes with a green checkmark,
  3. click on the "Fenix - assemble" task, then click "Run Artifacts".
  4. the APK links should be on the left side of the screen, named for each CPU architecture

@gabrielluong gabrielluong added the needs:review PRs that need to be reviewed label Oct 16, 2020
@gabrielluong gabrielluong requested review from a team as code owners October 16, 2020 19:01
eliserichards
eliserichards approved these changes Oct 16, 2020
View reviewed changes
Copy link
Contributor

@eliserichards eliserichards left a comment

Choose a reason for hiding this comment

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

Thank you! 🔥

@codecov-io
Copy link

codecov-io commented Oct 16, 2020 •
edited

Codecov Report

Merging #15975 into master will increase coverage by 0.03%.
The diff coverage is 3.84%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #15975      +/-   ##
============================================
+ Coverage     29.81%   29.85%   +0.03%     
  Complexity     1190     1190              
============================================
  Files           455      453       -2     
  Lines         18567    18543      -24     
  Branches       2409     2407       -2     
============================================
  Hits           5536     5536              
+ Misses        12636    12613      -23     
+ Partials        395      394       -1
Impacted Files Coverage Δ Complexity Δ
...lla/fenix/components/toolbar/ToolbarIntegration.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...zilla/fenix/settings/advanced/LocaleViewHolders.kt 100.00% <100.00%> (ø) 0.00 <0.00> (ø)
...pp/src/main/java/org/mozilla/fenix/FeatureFlags.kt 100.00% <0.00%> (ø) 5.00% <0.00%> (ø%)
...c/main/java/org/mozilla/fenix/home/HomeFragment.kt 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
.../java/org/mozilla/fenix/browser/BrowserFragment.kt 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
...in/java/org/mozilla/fenix/gestures/GestureUtils.kt
...a/org/mozilla/fenix/gestures/SwipeGestureLayout.kt
...mozilla/fenix/browser/SwitchTabsGestureListener.kt
...zilla/fenix/gestures/ShowTabTrayGestureListener.kt
...org/mozilla/fenix/browser/ToolbarGestureHandler.kt 0.00% <0.00%> (ø) 0.00% <0.00%> (?%)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d208ffe...d95b6f7. Read the comment docs.

@gabrielluong gabrielluong added pr:approved PR that has been approved pr:waiting-for-authors PR that has been approved and awaiting any changes before they can land and removed needs:review PRs that need to be reviewed labels Oct 16, 2020
@gabrielluong
Copy link
Member Author

Gonna add a comment before landing.

@gabrielluong gabrielluong added pr:needs-landing PRs that are ready to land [Will be merged by Mergify] and removed pr:do-not-land pr:waiting-for-authors PR that has been approved and awaiting any changes before they can land labels Oct 16, 2020
ekager
ekager reviewed Oct 16, 2020
View reviewed changes
app/src/main/java/org/mozilla/fenix/settings/advanced/LocaleViewHolders.kt
@@ -20,8 +20,9 @@ class LocaleViewHolder(
) : BaseLocaleViewHolder(view, selectedLocale) {

override fun bind(locale: Locale) {
// capitalisation is done using the rules of the appropriate locale (endonym and exonym)
// Capitalisation is done using the rules of the appropriate locale (endonym and exonym).
Copy link
Contributor

Choose a reason for hiding this comment

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

ironic that we're capitalizing capitalisation

Copy link
Contributor

Choose a reason for hiding this comment

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

😎

@gabrielluong gabrielluong reopened this Oct 16, 2020
@gabrielluong gabrielluong reopened this Oct 17, 2020
@gabrielluong gabrielluong reopened this Oct 17, 2020
@gabrielluong gabrielluong merged commit d33bd76 into mozilla-mobile:master Oct 17, 2020
@gabrielluong gabrielluong deleted the locale branch October 17, 2020 13:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ekager ekager ekager left review comments

@eliserichards eliserichards eliserichards approved these changes

Assignees

@eliserichards eliserichards

Labels
pr:approved PR that has been approved pr:needs-landing PRs that are ready to land [Will be merged by Mergify]
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Show selected language in "Follow device language" summary
4 participants
@gabrielluong @codecov-io @ekager @eliserichards