Skip to content

/ ghostery-extension

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

Jump to bottom

GH-1607/Remove feature listing #452

Merged
merged 6 commits into from Sep 25, 2019
+15 −5
Merged

GH-1607/Remove feature listing #452

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
65df03f
Remove feature listing for ghostery rewards on Cliqz browser
benstrumeyer Sep 18, 2019
62b92e3
Refactored menu items to a constant
benstrumeyer Sep 18, 2019
b37308e
Refactored menuItems. Removed Ghostery Rewards reference in Cliqz bro…
benstrumeyer Sep 20, 2019
bb92e84
Refactored to remove a variable
benstrumeyer Sep 20, 2019
2e0e65f
Merge branch 'develop' into feature/remove-feature-listing
benstrumeyer Sep 20, 2019
5113452
Refactor switch to if/else. Modify language.
benstrumeyer Sep 20, 2019
File filter
Filter file types
Clear filters
Jump to
Jump to file
Failed to load files.

Always

Just for now

Prev
Loading status checks…

Refactor switch to if/else. Modify language.

  • Loading branch information
@benstrumeyer
benstrumeyer committed Sep 20, 2019
commit 511345293cf2334b8c2b2e2b2768010d79a9b340
2 _locales/en/messages.json
View file
Open in desktop
@@ -1396,7 +1396,7 @@
"message": "New users are opted in to participating in Human Web and Ghostery Rewards. You may change these settings in custom setup or in the extension. "
},
"hub_home_header_info_cliqz": {
"message": "New users are opted in to participating in Human Web. You may change these settings in custom setup or in the extension. "
"message": "New users are opted in to participating in Human Web. You may change this setting in custom setup or in the extension. "
},
This conversation was marked as resolved by benstrumeyer

This comment has been minimized.

Sign in to view
@wlycdgr

wlycdgr Sep 20, 2019
Member

Since we only mention one setting for Cliqz, it should be 'this setting' in the second sentence here.

This comment has been minimized.

Sign in to view
@benstrumeyer

benstrumeyer Sep 20, 2019
Author Contributor

Gotcha, fixed.
"hub_home_header_info_link": {
"message": "Learn More."
15 app/hub/Views/HomeView/HomeView.jsx
View file
Open in desktop
@@ -35,17 +35,12 @@ const HomeView = (props) => {
} = props;
const accountHref = `https://account.${globals.GHOSTERY_DOMAIN}.com`;

let headerInfoText;
let headerInfoText = t('hub_home_header_info');
if (globals.BROWSER_INFO) {
switch (globals.BROWSER_INFO.name) {
case 'firefox':
headerInfoText = t('hub_home_header_info_opted_out');
break;
case 'cliqz':
headerInfoText = t('hub_home_header_info_cliqz');
break;
default:
headerInfoText = t('hub_home_header_info');
if (globals.BROWSER_INFO.name === 'firefox') {
headerInfoText = t('hub_home_header_info_opted_out');
} else if (globals.BROWSER_INFO.name === 'cliqz') {
headerInfoText = t('hub_home_header_info_cliqz');
}
}

This conversation was marked as resolved by benstrumeyer

This comment has been minimized.

Sign in to view
@wlycdgr

wlycdgr Sep 20, 2019
Member

This introduces a dependency on globals.BROWSER_INFO being set compared to the old code; let's refactor to avoid doing that. We can do

let headerInfoText = t('hub_home_header_info');
if (globals.BROWSER_INFO) {
    // switching to if/else to avoid duplication in the default case of switch
    if (globals.BROWSER_INFO.name === 'firefox') { headerInfoText = ....}
    else if (.... === 'cliqz') { .....}
}

This comment has been minimized.

Sign in to view
@benstrumeyer

benstrumeyer Sep 20, 2019
Author Contributor

Great catch. Fixed.
ProTip! Use n and p to navigate between commits in a pull request.