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.

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

combine trackers blocked and ads blocked on NTP #2950

Merged
merged 1 commit into from
Jul 24, 2019
Merged

Conversation

cezaraugusto
Copy link
Contributor

close brave/brave-browser#5273

Test Plan:

  1. Before checking out this branch, open NTP and check the number of ads and trackers blocked
  2. Check out this branch, open NTP and assert the current value is the sum of your previous stats of ads and trackers

The combination of ads and trackers is now called "trackers blocked" only, per brave/brave-browser#5273 (comment).

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

imptrx
imptrx previously approved these changes Jul 18, 2019
Copy link
Contributor

@imptrx imptrx left a comment

Choose a reason for hiding this comment

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

Tested and verified that ads and trackers blocked are now aggregated into one number - checkouts on storybooks too

nit: while we are on this issue, I was wondering if it be possible to keep #B02FFB (purple) as the color rather than #4C54D2 (blue) IMO the blue is hard to differentiate with certain background images and this holds especially true with the gradient 😅

So:
Screen Shot 2019-07-18 at 3 13 40 PM

Rather than:
Screen Shot 2019-07-18 at 2 52 43 PM

@cezaraugusto
Copy link
Contributor Author

@imptrx good one. @karenkliu thoughts on #2950 (review)?

@karenkliu
Copy link

@imptrx Please set the label to be "Ads and Trackers blocked" for now and use #A0A5EB (light purple) instead of the blue. (This is a stopgap measure until we work on fixing the labels so that it matches the Shields panel labels).

@rebron
Copy link
Collaborator

rebron commented Jul 19, 2019

Let's stick with "Ads and Trackers blocked" for now as @karenkliu mentioned and can revisit labels to match shields later. We'll surely get questions on the removal of "Ads blocked" and fewer questions if simply combining the labels.

@cezaraugusto
Copy link
Contributor Author

updated

Copy link
Contributor

@imptrx imptrx left a comment

Choose a reason for hiding this comment

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

Verified new text and combined tracker 🛳️🇮🇹

@cezaraugusto cezaraugusto merged commit a31cc1f into master Jul 24, 2019
@cezaraugusto cezaraugusto added this to the 0.70.x - Nightly milestone Jul 24, 2019
@imptrx imptrx deleted the ca-5273 branch July 24, 2019 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

combine Trackers Blocked and Ads Blocked on NTP
4 participants