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-1607/Remove feature listing #452

Merged
merged 6 commits into from Sep 25, 2019

Refactored menuItems. Removed Ghostery Rewards reference in Cliqz bro…

…wser Hub
  • Loading branch information
benstrumeyer committed Sep 20, 2019
commit b37308e69d44c0d4da210c974ba74917c5d28a71
@@ -1395,6 +1395,9 @@
"hub_home_header_info": {
"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. "
},
This conversation was marked as resolved by benstrumeyer

This comment has been minimized.

@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.

@benstrumeyer

benstrumeyer Sep 20, 2019
Author Contributor

Gotcha, fixed.

"hub_home_header_info_link": {
"message": "Learn More."
},
@@ -34,9 +34,22 @@ const HomeView = (props) => {
isPlus,
} = props;
const accountHref = `https://account.${globals.GHOSTERY_DOMAIN}.com`;
const headerInfoText = (globals.BROWSER_INFO && globals.BROWSER_INFO.name === 'firefox')
? t('hub_home_header_info_opted_out')
: t('hub_home_header_info');
const browserName = globals.BROWSER_INFO.name;

let headerInfoText;
if (globals.BROWSER_INFO) {
switch (browserName) {
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');
}
}

This conversation was marked as resolved by benstrumeyer

This comment has been minimized.

@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.

@benstrumeyer

benstrumeyer Sep 20, 2019
Author Contributor

Great catch. Fixed.

const tutorialFeatureClassNames = ClassNames('HomeView__onboardingFeature columns flex-container align-middle flex-dir-column', {
'feature-tutorial-complete': tutorial_complete,
'feature-tutorial': !tutorial_complete,
@@ -53,7 +53,7 @@ class SideNavigationViewContainer extends Component {
{ href: '/tutorial', icon: 'tutorial', text: t('hub_side_navigation_tutorial') },
{ href: '/plus', icon: 'plus', text: t('hub_side_navigation_supporter') },
...(IS_CLIQZ ? [] : [{ href: '/rewards', icon: 'rewards', text: t('hub_side_navigation_rewards') }]),
...[{ href: '/products', icon: 'products', text: t('hub_side_navigation_products') }]
{ href: '/products', icon: 'products', text: t('hub_side_navigation_products') }
];
const bottomItems = user ? [
{ id: 'email', href: `https://account.${globals.GHOSTERY_DOMAIN}.com/`, text: user.email },
ProTip! Use n and p to navigate between commits in a pull request.