-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Closes #16345: Standartizing share icon across all app #16536
Conversation
replace mozac_ic_share for ic_hollow_share on DefaultToolbarMenu
updating share icon to a filled shape, like default toolbar menu across all app
No Taskcluster jobs started for this pull requestThe `allowPullRequests` configuration for this repository (in `.taskcluster.yml` on the
default branch) does not allow starting tasks for this pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we just remove the ic_hollow_share
altogether and use the AC mozac_ic_share
when needed? The only drawback I can think of is we need to make sure it's tinted correctly where used.
contentDescription = context.getString(R.string.browser_menu_share), | ||
iconTintColorResource = primaryTextColor(), | ||
listener = { | ||
onItemTapped.invoke(ToolbarMenu.Item.Share) | ||
} | ||
) | ||
|
||
registerForIsBookmarkedUpdates() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we removing this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't know, putting it back
You're right, but |
… for bookmark changes comment link: mozilla-mobile#16536 (comment)
bors try |
tryBuild succeeded: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I'm fine with changing the asset!
bors r+ |
16536: Closes #16345: Standartizing share icon across all app r=ekager a=LeoMaldonado ### Pull Request checklist <!-- Before submitting the PR, please address each item --> - [x] **Tests**: This PR includes thorough tests or an explanation of why it does not - [x] **Screenshots**: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not - [x] **Accessibility**: The code in this PR follows [accessibility best practices](https://github.com/mozilla-mobile/shared-docs/blob/master/android/accessibility_guide.md) or does not include any user facing features. In addition, it includes a screenshot of a successful [accessibility scan](https://play.google.com/store/apps/details?id=com.google.android.apps.accessibility.auditor&hl=en_US) 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 This PR closes #16345. I removed share icon outlined by a filled share icon and put it in all references to share icons. ![Screenshot_20201112-165052](https://user-images.githubusercontent.com/1688902/98990102-cb8e5180-2508-11eb-9936-fc254c90b407.png) ![Screenshot_20201112-165153](https://user-images.githubusercontent.com/1688902/98990107-ccbf7e80-2508-11eb-826f-cd8451b70acc.png) ![Screenshot_20201112-165258](https://user-images.githubusercontent.com/1688902/98990109-cd581500-2508-11eb-93e9-c43e24403a5a.png) Co-authored-by: LeoMaldonado <leomaldonadopassos@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #16536 +/- ##
=========================================
Coverage 29.93% 29.93%
Complexity 1216 1216
=========================================
Files 455 455
Lines 18696 18696
Branches 2571 2571
=========================================
Hits 5596 5596
Misses 12665 12665
Partials 435 435
Continue to review full report at Codecov.
|
Merged, thanks for the contribution! |
Pull Request checklist
To download an APK when reviewing a PR:
This PR closes #16345. I removed share icon outlined by a filled share icon and put it in all references to share icons.