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

GH-2178: Update Anti-suite names where they appear in GBE #614

Closed
wants to merge 16 commits into from

Conversation

@benstrumeyer
Copy link
Contributor

@benstrumeyer benstrumeyer commented Oct 1, 2020

  • Have you followed the guidelines in CONTRIBUTING.md?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you added an explanation of what your changes do?
  • Does your submission pass tests?
  • Did you lint your code prior to submission?

Change string names from:

  • Enhanced Anti-Tracking → Anti-Tracking
  • Enhanced Ad Blocking → Ad-Blocking
  • Smart Blocking → Smart Browsing
  • Ad Block / Ad Block Lists → Ad-Block / Ad-Block Lists

Ticket: https://ghostery.atlassian.net/browse/GH-2178

benstrumeyer added 2 commits Oct 1, 2020
…ock lists to their new names
@benstrumeyer benstrumeyer added this to the 8.5.4 milestone Oct 1, 2020
@benstrumeyer benstrumeyer requested a review from wlycdgr Oct 1, 2020
@benstrumeyer benstrumeyer requested a review from ghostery/extension as a code owner Oct 1, 2020
@benstrumeyer benstrumeyer self-assigned this Oct 1, 2020
Copy link
Member

@wlycdgr wlycdgr left a comment

Looks good, just a couple other spots that need to be updated

@@ -63,12 +63,12 @@
background-image: buildAdBlockActiveIcon(#1dafed);
}

// Smart Blocking Icon
// Smart Browsing Icon
.SetupAntiSuite__feature .smart-blocking {

This comment has been minimized.

@wlycdgr

wlycdgr Oct 1, 2020
Member

Let's also change the class names .smart-blocking -> .smart-browsing

This comment has been minimized.

@benstrumeyer

benstrumeyer Oct 1, 2020
Author Contributor

Updated the class names!

app/panel/components/BuildingBlocks/CliqzFeature.jsx Outdated Show resolved Hide resolved
} else if (featureType === 'smart_block') {
featureName = t('smart_blocking');
}
Comment on lines 143 to 145

This comment has been minimized.

@wlycdgr

wlycdgr Oct 1, 2020
Member

And here

This comment has been minimized.

@benstrumeyer

benstrumeyer Oct 1, 2020
Author Contributor

Updated!

@benstrumeyer benstrumeyer requested a review from wlycdgr Oct 1, 2020
@wlycdgr wlycdgr changed the title GH-2178: Update Anti-suite names where they appear in GBE (DNM UNTIL 8.5.3 IS OUT) GH-2178: Update Anti-suite names where they appear in GBE Oct 2, 2020
Copy link
Member

@wlycdgr wlycdgr left a comment

Quick comment on one of the updates. Otherwise, we were looking good until I made the mistake of running a global search on 'smart_block'....looks like there are....a few more spots:

image

I know it's a pain, but let's update those and try to stick closer to one name per feature - we have enough confusion around feature names in the extension as is

// Smart Browsing Icon
.SetupAntiSuite__feature .smart-blocking {
.SetupAntiSuite__feature .smart-browsing {
background-image: buildSmartBrowsingIcon(#9f9f9f);
}
.SetupAntiSuite__feature .smart-blocking.active {
.SetupAntiSuite__feature .smart-browsing.active {
background-image: buildSmartBrowsingActiveIcon(#1dafed);
Comment on lines 66 to 71

This comment has been minimized.

@wlycdgr

wlycdgr Oct 2, 2020
Member

Did you also update the place(s) in the components where these classes are used? Sorry if I missed that

This comment has been minimized.

@benstrumeyer

benstrumeyer Oct 6, 2020
Author Contributor

My bad! Looks like it got updated by the global search and replace 👍

benstrumeyer added 2 commits Oct 6, 2020
@benstrumeyer benstrumeyer requested a review from christophertino as a code owner Oct 6, 2020
@jmlh556
Copy link

@jmlh556 jmlh556 commented Oct 6, 2020

@benstrumeyer benstrumeyer requested a review from wlycdgr Oct 6, 2020
@wlycdgr wlycdgr changed the title (DNM UNTIL 8.5.3 IS OUT) GH-2178: Update Anti-suite names where they appear in GBE (8.5.4) GH-2178: Update Anti-suite names where they appear in GBE Oct 7, 2020
@wlycdgr
Copy link
Member

@wlycdgr wlycdgr commented Oct 27, 2020

Looks like there's supposed to be a dash between "Smart" and "Browsing"

@wlycdgr
Copy link
Member

@wlycdgr wlycdgr commented Oct 27, 2020

"The following Ad-Block filter lists" should be "The following ad-block filter lists" (in the Ad-Block Lists Settings tab)

@@ -134,7 +134,7 @@ See the complete GitHub [milestone](https://github.com/ghostery/ghostery-extensi
+ Add new counter for Requests Modified by Anti-Tracking (#392)
+ Show fingerprint, cookie and advertisement icons in Detail View tracker list (#394)
+ Improved Anti-Tracking integration (#377)
+ Integrate Click2Play into SmartBlocking (#388)
+ Integrate Click2Play into SmartBrowsing (#388)

This comment has been minimized.

@wlycdgr

wlycdgr Oct 27, 2020
Member

Same deal here with the CHANGELOG - let's leave the old entries as is

This comment has been minimized.

@benstrumeyer

benstrumeyer Oct 27, 2020
Author Contributor

Updated!

const smartBrowseed = !block ? this.policySmartBrowse.shouldBlock(app_id, cat_id, tab_id, page_url, eventMutable.type, eventMutable.timeStamp) : false;
const smartUnblocked = block ? this.policySmartBrowse.shouldUnblock(app_id, cat_id, tab_id, page_url, eventMutable.type) : false;
Comment on lines 421 to 422

This comment has been minimized.

@wlycdgr

wlycdgr Oct 27, 2020
Member

Looks like a spot where the auto-replace didn't quite do the trick. Let's update these to smartBrowseBlocked and smartBrowseUnblocked

This comment has been minimized.

@benstrumeyer

benstrumeyer Oct 27, 2020
Author Contributor

Typos! Gotta watch out for those, good catch

@@ -15,7 +15,7 @@
* prefetched: {boolean} indicates that the tab was prefetched and not part of the main window
* purplebox: {boolean} indicates that the purplebox.js script has been loaded on this tab
* protocol: {string} request protocol
* smartBlock: {Object} smart blocking stats for this tab
* smartBrowse: {Object} smart blocking stats for this tab

This comment has been minimized.

@wlycdgr

wlycdgr Oct 27, 2020
Member

Comment needs an update to match ~

This comment has been minimized.

@benstrumeyer

benstrumeyer Oct 27, 2020
Author Contributor

Nice catch! I just found/replaced the variables, didn't think to look for comments too. Updated the comments in a few more places too

@benstrumeyer benstrumeyer requested a review from wlycdgr Oct 27, 2020
@@ -119,7 +119,7 @@ class ConfData {
_initProperty('enable_click2play_social', true);
_initProperty('enable_human_web', !IS_CLIQZ && !IS_FIREFOX);
_initProperty('enable_abtests', true);
_initProperty('enable_smart_block', true);
_initProperty('enable_smart_browse', true);

This comment has been minimized.

@christophertino

christophertino Oct 27, 2020
Member

This is going to override the setting for any users with Smart Block currently turned off. Is there a reason we are changing all of these variable and file names, instead of just changing the user-facing language?

@christophertino christophertino removed this from the 8.5.4 milestone Oct 27, 2020
@christophertino
Copy link
Member

@christophertino christophertino commented Oct 27, 2020

Closing in favor of #626

@christophertino christophertino deleted the GH-2178 branch Oct 27, 2020
@christophertino christophertino changed the title (8.5.4) GH-2178: Update Anti-suite names where they appear in GBE GH-2178: Update Anti-suite names where they appear in GBE Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants