Skip to content
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

Update notification icons #3670

Merged
merged 2 commits into from
Oct 20, 2023
Merged

Update notification icons #3670

merged 2 commits into from
Oct 20, 2023

Conversation

CrisBarreiro
Copy link
Contributor

@CrisBarreiro CrisBarreiro commented Oct 13, 2023 •
edited
Loading

Task/Issue URL: https://app.asana.com/0/0/1205217801169321/f

Description

See title

Steps to test this PR

Feature 1

  • Give the app notification permissions
  • Enable AppTP
  • Check icons on the notification

UI changes

notificationdisabled
notificationafter
bannerafter
bannerenabled

This was referenced Oct 13, 2023
Merged
Merged
@CrisBarreiro
Copy link
Contributor Author

CrisBarreiro commented Oct 13, 2023 •
edited
Loading

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

This was referenced Oct 13, 2023
Closed
Merged
@CrisBarreiro CrisBarreiro force-pushed the feature/cbarreiro/change-copy-my-apps branch 2 times, most recently from 51775e5 to 4bf5ccc Compare October 13, 2023 15:49
@CrisBarreiro CrisBarreiro force-pushed the feature/cbarreiro/update-notification-icons branch from 12dfa69 to ebb8f55 Compare October 13, 2023 15:49
@CrisBarreiro CrisBarreiro force-pushed the feature/cbarreiro/change-copy-my-apps-l10n branch from 4bf5ccc to ee1cfff Compare October 13, 2023 16:10
@CrisBarreiro CrisBarreiro assigned aitorvs and unassigned aitorvs Oct 16, 2023
@CrisBarreiro CrisBarreiro changed the base branch from feature/cbarreiro/change-copy-my-apps-l10n to feature/cbarreiro/remove-info-message-enabled October 16, 2023 07:30
@CrisBarreiro CrisBarreiro force-pushed the feature/cbarreiro/update-notification-icons branch from ebb8f55 to c054146 Compare October 16, 2023 07:32
This was referenced Oct 16, 2023
Merged
Merged
Merged
@CrisBarreiro CrisBarreiro mentioned this pull request Oct 16, 2023
Merged
3 tasks
aitorvs
aitorvs approved these changes Oct 16, 2023
View reviewed changes
Copy link
Collaborator

@aitorvs aitorvs left a comment

Choose a reason for hiding this comment

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

The size of the icons change, I guess that's expected?

@CrisBarreiro CrisBarreiro force-pushed the feature/cbarreiro/remove-info-message-enabled branch from 285d095 to 105edcf Compare October 18, 2023 07:16
@CrisBarreiro CrisBarreiro force-pushed the feature/cbarreiro/update-notification-icons branch from c054146 to 46ee896 Compare October 18, 2023 07:17
@CrisBarreiro CrisBarreiro force-pushed the feature/cbarreiro/remove-info-message-enabled branch from 105edcf to 4ed4312 Compare October 18, 2023 14:31
@CrisBarreiro CrisBarreiro force-pushed the feature/cbarreiro/update-notification-icons branch from 46ee896 to 96d5eeb Compare October 18, 2023 14:31
@CrisBarreiro CrisBarreiro mentioned this pull request Oct 18, 2023
Merged
14 tasks
@CrisBarreiro CrisBarreiro force-pushed the feature/cbarreiro/remove-info-message-enabled branch from 4ed4312 to c36d300 Compare October 19, 2023 11:20
@CrisBarreiro CrisBarreiro force-pushed the feature/cbarreiro/update-notification-icons branch 2 times, most recently from 3935c51 to 6fef5cf Compare October 19, 2023 11:34
@CrisBarreiro CrisBarreiro force-pushed the feature/cbarreiro/remove-info-message-enabled branch from c36d300 to 1d8571a Compare October 19, 2023 11:57
@CrisBarreiro CrisBarreiro force-pushed the feature/cbarreiro/update-notification-icons branch from 6fef5cf to a264019 Compare October 19, 2023 11:57
@CrisBarreiro CrisBarreiro force-pushed the feature/cbarreiro/update-notification-icons branch from a264019 to a9dcc47 Compare October 19, 2023 12:04
@CrisBarreiro
Copy link
Contributor Author

The size of the icons change, I guess that's expected?

It was expected, but it seems to be causing issues in some devices, so I kept the original size

@CrisBarreiro
Update known to collect icons (#3671)
b451821 
<!--
Note: This checklist is a reminder of our shared engineering
expectations.
The items in Bold are required
If your PR involves UI changes:
1. Upload screenshots or screencasts that illustrate the changes before
/ after
2. Add them under the UI changes section (feel free to add more columns
if needed)
If your PR does not involve UI changes, you can remove the **UI
changes** section

At a minimum, make sure your changes are tested in API 23 and one of the
more recent API levels available.
-->

Task/Issue URL: https://app.asana.com/0/0/1205176011518410/f 

### Description
Update icons in "Known to collect" 

### Steps to test this PR

_Feature 1_
- [ ] Enable AppTP
- [x] Open an app that uses trackers
- [x] Get back to AppTP and check blocked trackers
- [x] Click on a specific trackers
- [x] Check we're displaying the new icons

### UI changes

![knowncollect](https://github.com/duckduckgo/Android/assets/6297834/449f6151-f85f-415d-93ff-391aa3edb1b7)
@CrisBarreiro CrisBarreiro merged commit 42a11a0 into feature/cbarreiro/remove-info-message-enabled Oct 20, 2023
@CrisBarreiro CrisBarreiro deleted the feature/cbarreiro/update-notification-icons branch October 20, 2023 12:00
CrisBarreiro added a commit that referenced this pull request Oct 20, 2023
@CrisBarreiro
Update notification icons (#3670)
f1a7016 
<!--
Note: This checklist is a reminder of our shared engineering
expectations.
The items in Bold are required
If your PR involves UI changes:
1. Upload screenshots or screencasts that illustrate the changes before
/ after
2. Add them under the UI changes section (feel free to add more columns
if needed)
If your PR does not involve UI changes, you can remove the **UI
changes** section

At a minimum, make sure your changes are tested in API 23 and one of the
more recent API levels available.
-->

Task/Issue URL: https://app.asana.com/0/0/1205217801169321/f 

### Description
See title

### Steps to test this PR

_Feature 1_
- [ ] Give the app notification permissions
- [ ] Enable AppTP
- [ ] Check icons on the notification

### UI changes

![notificationdisabled](https://github.com/duckduckgo/Android/assets/6297834/1e98b17d-82e0-4099-bfb1-1a6f355b72fd)

![notificationafter](https://github.com/duckduckgo/Android/assets/6297834/2b663cd2-3877-43af-bbcc-a475309ee087)

![bannerafter](https://github.com/duckduckgo/Android/assets/6297834/e120178a-a93d-49d4-ba9c-c19fcb81f292)

![bannerenabled](https://github.com/duckduckgo/Android/assets/6297834/56e07437-5de9-4759-9f8e-b364b1f4b2b1)
@CrisBarreiro CrisBarreiro mentioned this pull request Oct 20, 2023
Merged
CrisBarreiro added a commit that referenced this pull request Oct 25, 2023
@CrisBarreiro
Update notification icons (#3670)
34468a2 
<!--
Note: This checklist is a reminder of our shared engineering
expectations.
The items in Bold are required
If your PR involves UI changes:
1. Upload screenshots or screencasts that illustrate the changes before
/ after
2. Add them under the UI changes section (feel free to add more columns
if needed)
If your PR does not involve UI changes, you can remove the **UI
changes** section

At a minimum, make sure your changes are tested in API 23 and one of the
more recent API levels available.
-->

Task/Issue URL: https://app.asana.com/0/0/1205217801169321/f 

### Description
See title

### Steps to test this PR

_Feature 1_
- [ ] Give the app notification permissions
- [ ] Enable AppTP
- [ ] Check icons on the notification

### UI changes

![notificationdisabled](https://github.com/duckduckgo/Android/assets/6297834/1e98b17d-82e0-4099-bfb1-1a6f355b72fd)

![notificationafter](https://github.com/duckduckgo/Android/assets/6297834/2b663cd2-3877-43af-bbcc-a475309ee087)

![bannerafter](https://github.com/duckduckgo/Android/assets/6297834/e120178a-a93d-49d4-ba9c-c19fcb81f292)

![bannerenabled](https://github.com/duckduckgo/Android/assets/6297834/56e07437-5de9-4759-9f8e-b364b1f4b2b1)
CrisBarreiro added a commit that referenced this pull request Nov 6, 2023
@CrisBarreiro
AppTP - minor UX improvements (#3668)
4bf1f70 
* **PR #3670** <a
href="https://app.graphite.dev/github/pr/duckduckgo/Android/3670"
target="_blank"><img
src="https://static.graphite.dev/graphite-32x32-black.png"
alt="Graphite" width="10px" height="10px"/></a>
* **PR #3671** <a
href="https://app.graphite.dev/github/pr/duckduckgo/Android/3671"
target="_blank"><img
src="https://static.graphite.dev/graphite-32x32-black.png"
alt="Graphite" width="10px" height="10px"/></a>
* **PR #3683** <a
href="https://app.graphite.dev/github/pr/duckduckgo/Android/3683"
target="_blank"><img
src="https://static.graphite.dev/graphite-32x32-black.png"
alt="Graphite" width="10px" height="10px"/></a>
* **PR #3684** <a
href="https://app.graphite.dev/github/pr/duckduckgo/Android/3684"
target="_blank"><img
src="https://static.graphite.dev/graphite-32x32-black.png"
alt="Graphite" width="10px" height="10px"/></a>
* **PR #3686** <a
href="https://app.graphite.dev/github/pr/duckduckgo/Android/3686"
target="_blank"><img
src="https://static.graphite.dev/graphite-32x32-black.png"
alt="Graphite" width="10px" height="10px"/></a>
* **PR #3693** <a
href="https://app.graphite.dev/github/pr/duckduckgo/Android/3693"
target="_blank"><img
src="https://static.graphite.dev/graphite-32x32-black.png"
alt="Graphite" width="10px" height="10px"/></a>
* **PR #3715** <a
href="https://app.graphite.dev/github/pr/duckduckgo/Android/3715"
target="_blank"><img
src="https://static.graphite.dev/graphite-32x32-black.png"
alt="Graphite" width="10px" height="10px"/></a>
* **PR #3732** <a
href="https://app.graphite.dev/github/pr/duckduckgo/Android/3732"
target="_blank"><img
src="https://static.graphite.dev/graphite-32x32-black.png"
alt="Graphite" width="10px" height="10px"/></a>
joshliebe pushed a commit that referenced this pull request Nov 7, 2023
@CrisBarreiro @joshliebe
AppTP - minor UX improvements (#3668)
915e1e6 
* **PR #3670** <a
href="https://app.graphite.dev/github/pr/duckduckgo/Android/3670"
target="_blank"><img
src="https://static.graphite.dev/graphite-32x32-black.png"
alt="Graphite" width="10px" height="10px"/></a>
* **PR #3671** <a
href="https://app.graphite.dev/github/pr/duckduckgo/Android/3671"
target="_blank"><img
src="https://static.graphite.dev/graphite-32x32-black.png"
alt="Graphite" width="10px" height="10px"/></a>
* **PR #3683** <a
href="https://app.graphite.dev/github/pr/duckduckgo/Android/3683"
target="_blank"><img
src="https://static.graphite.dev/graphite-32x32-black.png"
alt="Graphite" width="10px" height="10px"/></a>
* **PR #3684** <a
href="https://app.graphite.dev/github/pr/duckduckgo/Android/3684"
target="_blank"><img
src="https://static.graphite.dev/graphite-32x32-black.png"
alt="Graphite" width="10px" height="10px"/></a>
* **PR #3686** <a
href="https://app.graphite.dev/github/pr/duckduckgo/Android/3686"
target="_blank"><img
src="https://static.graphite.dev/graphite-32x32-black.png"
alt="Graphite" width="10px" height="10px"/></a>
* **PR #3693** <a
href="https://app.graphite.dev/github/pr/duckduckgo/Android/3693"
target="_blank"><img
src="https://static.graphite.dev/graphite-32x32-black.png"
alt="Graphite" width="10px" height="10px"/></a>
* **PR #3715** <a
href="https://app.graphite.dev/github/pr/duckduckgo/Android/3715"
target="_blank"><img
src="https://static.graphite.dev/graphite-32x32-black.png"
alt="Graphite" width="10px" height="10px"/></a>
* **PR #3732** <a
href="https://app.graphite.dev/github/pr/duckduckgo/Android/3732"
target="_blank"><img
src="https://static.graphite.dev/graphite-32x32-black.png"
alt="Graphite" width="10px" height="10px"/></a>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aitorvs aitorvs aitorvs approved these changes

Assignees

@aitorvs aitorvs

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@CrisBarreiro @aitorvs