-
Notifications
You must be signed in to change notification settings - Fork 921
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
Don't show animation of content setting image at all #2639
Conversation
b4540ee
to
975637c
Compare
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.
we can get rid of the entire autoplay animation by removing this line
set_explanatory_string_id(IDS_BLOCKED_AUTOPLAY_TITLE); |
Widevine used to have this animation and it is removed
so why do we still show it for the first time if user find it annoying?
Asking for designer/pm input now... cc: @bradleyrichter @rebron |
975637c
to
1909b12
Compare
I discussed with @bradleyrichter. Let's remove slide animation altogether, don't even show for first time use. |
1909b12
to
5c6cd0b
Compare
Frequent slide animation distracts users.
5c6cd0b
to
a5e5467
Compare
@darkdh slide animations for all content settings are disabled. PTAL again. |
Awesome job on this one, @simonhong! Thanks for the quick review, @darkdh 😄 |
@the-4-limbos yes, this change is not yet in current stable(0.66). It is int dev channel (0.68.x) |
call `Initialize` at the time that brave ads is disabled, not on shut…
call `Initialize` at the time that brave ads is disabled, not on shut…
Frequent slide animation distracts users.
Fix brave/brave-browser#3751
Submitter Checklist:
npm test brave_unit_tests && npm test brave_browser_tests && npm run test-security
) onnpm run lint
)git rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
See brave/brave-browser#3751 for STR.
Reviewer Checklist:
After-merge Checklist:
changes has landed on.