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

For #12533: Align main settings categories according to specs. #12534

Merged
merged 1 commit into from
Jul 29, 2020
Merged

For #12533: Align main settings categories according to specs. #12534

merged 1 commit into from
Jul 29, 2020

Conversation

mcarare
Copy link
Contributor

@mcarare mcarare commented Jul 14, 2020

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.
    Screenshot_20200714_101141_org mozilla fenix debug

After merge

  • Milestone: Make sure issues finished by this pull request are added to the milestone of the version currently in development.

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

@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2020 •
edited
Loading

Codecov Report

Merging #12534 into master will decrease coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #12534      +/-   ##
============================================
- Coverage     23.66%   23.60%   -0.06%     
  Complexity      798      798              
============================================
  Files           377      377              
  Lines         15478    15478              
  Branches       2020     2020              
============================================
- Hits           3663     3654       -9     
- Misses        11507    11515       +8     
- Partials        308      309       +1
Impacted Files Coverage Δ Complexity Δ
...mponents/searchengine/FenixSearchEngineProvider.kt 58.55% <0.00%> (-4.51%) 14.00% <0.00%> (ø%)
...nix/components/toolbar/BrowserToolbarController.kt 67.26% <0.00%> (-2.39%) 0.00% <0.00%> (ø%)

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 67758ab...d50c679. Read the comment docs.

eliserichards
eliserichards suggested changes Jul 17, 2020
View reviewed changes
Copy link

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

I know you didn't choose the original name, but could we change this to "preference_category_main_style" or "preference_main_style"? 😅 🐈 🐱

@mcarare
Copy link
Contributor Author

mcarare commented Jul 20, 2020

@eliserichards I do not think preference_main_style would be relevant, as the style is applied to categories, not to individual preferences in settings screen.

@mcarare mcarare requested a review from eliserichards July 21, 2020 09:23
@mcarare
Copy link
Contributor Author

mcarare commented Jul 24, 2020

@eliserichards Can you re-review this? TY!

@cadeyrn
Copy link
Contributor

cadeyrn commented Jul 25, 2020

It would be great if @brampitoyo could comment on the open question in #12533 before landing this.

eliserichards
eliserichards approved these changes Jul 29, 2020
View reviewed changes
Copy link

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

Looks great! 🥇

@eliserichards eliserichards merged commit ac87063 into mozilla-mobile:master Jul 29, 2020
@mcarare mcarare deleted the 12533 branch October 21, 2020 09:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@eliserichards eliserichards eliserichards approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mcarare @codecov-commenter @cadeyrn @eliserichards