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

For #11132: Rearranges home menu items #11187

Merged
merged 2 commits into from
Jun 10, 2020

Conversation

ValentinTimisica
Copy link
Contributor

Pull Request checklist

  • Tests: This PR does not include thorough tests.
  • Screenshots: This PR includes screenshots of the changes made.
  • Accessibility: The code in this PR follows accessibility best practices.
Bottom toolbar Top toolbar

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

@ValentinTimisica ValentinTimisica added the needs:UX-feedback Needs UX Feedback label Jun 3, 2020
@codecov-commenter
Copy link

Codecov Report

Merging #11187 into master will decrease coverage by 0.00%.
The diff coverage is 7.69%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #11187      +/-   ##
============================================
- Coverage     20.45%   20.45%   -0.01%     
  Complexity      681      681              
============================================
  Files           372      372              
  Lines         15438    15440       +2     
  Branches       2081     2081              
============================================
  Hits           3158     3158              
- Misses        11984    11986       +2     
  Partials        296      296              
Impacted Files Coverage Δ Complexity Δ
...p/src/main/java/org/mozilla/fenix/home/HomeMenu.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...lla/fenix/components/toolbar/DefaultToolbarMenu.kt 43.31% <100.00%> (ø) 13.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 7013e81...c1eff88. Read the comment docs.

@Scripterr
Copy link

The add-ons menu icon looks bigger than other icons, it should be consistent with other icons.

@AmyYLee
Copy link
Collaborator

AmyYLee commented Jun 3, 2020

Assigned to @brampitoyo

@ValentinTimisica
Copy link
Contributor Author

ValentinTimisica commented Jun 4, 2020

@Scripterr thanks for pointing that out, but in this PR the add-ons icon was not replaced or modified. I looked over the code and, for example, add-ons icon has the same dimensions as the history icon. If you think this is a bug feel free to open a new issue with it.

@Scripterr
Copy link

I have filled the bug previously in #9205 , but it doesn't have any progress on it, so I saw this PR was changing some icons and menu positions that may be it can be acknowledged or fixed here.

@ekager
Copy link
Contributor

ekager commented Jun 4, 2020

I just bumped #9205 so hopefully we'll get new assets there.

@brampitoyo
Copy link

@ekager I’ve posted the new extensions icon asset over at #9205 (comment)

@brampitoyo brampitoyo added eng:ready Ready for engineering and removed needs:UX-feedback Needs UX Feedback labels Jun 7, 2020
@ekager
Copy link
Contributor

ekager commented Jun 10, 2020

We can follow up with the new assets later.

@ekager ekager merged commit 3a28704 into mozilla-mobile:master Jun 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
eng:ready Ready for engineering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants