-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New trackers animation #830
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.
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
New trackers animation #830
Conversation
…marcos/new_trackers_animation
…of github.com:duckduckgo/Android into feature/marcos/new_trackers_animation � Conflicts: � app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt � app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
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.
Still reviewing but I noticed somethin while testing (API 26):
- Visit any website cnn.com
- Tracker animation appears
- Go to privacy dashboard
- Disable privacy protection
- Go back to browser
- Animations will not run (👍 ) but privacy grade stays in the "pulsating state" (?)
|
@cmonfortep good catch with that bug! It should be now fixed :) |
cmonfortep
left a comment
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.
I'm astonished by this cool animation and all the UI work you did. I'm leaving some comments more related to how to represent the ViewState and the logic inside the fragment.
Happy to take it offline on Monday and jump into a zoom call 👍
app/src/main/java/com/duckduckgo/app/browser/BrowserTrackersAnimatorHelper.kt
Show resolved
Hide resolved
app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
Outdated
Show resolved
Hide resolved
cmonfortep
left a comment
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.
This is looking great, I only have two comments for me to understand better some of the logic.
app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
Outdated
Show resolved
Hide resolved
…of github.com:duckduckgo/Android into feature/marcos/new_trackers_animation � Conflicts: � app/src/androidTest/java/com/duckduckgo/app/trackerdetection/api/TdsEntityJsonTest.kt � app/src/main/java/com/duckduckgo/app/browser/BrowserTrackersAnimatorHelper.kt
cmonfortep
left a comment
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.
🚀 LGTM, tested on usual apis (21, 23, 26, 29) and it's working as expected 🎉
Task/Issue URL: https://app.asana.com/0/1125189844152671/1175120780820332
Tech Design URL:
CC:
Description:
This PR changes the trackers animation and fixes the privacy grade logos so they have the correct size. The new animation now has the privacy grade pulsating in a loading state and then the trackers logos slide in from the left, they get crossed and finally slide back in to the left. Once the animation finishes we show the privacy grade.
Steps to test this PR:
More than 3 trackers
Less than 3 trackers
3 trackers
Unknown logo
Cancelling the animation
Cancelling the animation 2
Privacy grade hides when the animation begins
Privacy grade shows after the animation finishes or when is cancelled
Internal references:
Software Engineering Expectations
Technical Design Template