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
@@ -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 this setting 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,16 @@ 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');

let headerInfoText = t('hub_home_header_info');
if (globals.BROWSER_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.

@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,
@@ -16,6 +16,7 @@ import PropTypes from 'prop-types';
import SideNavigationView from './SideNavigationView';
import globals from '../../../../src/classes/Globals';

const { IS_CLIQZ } = globals;
/**
* @class Implement the Side Navigation View for the Ghostery Hub
* @extends Component
@@ -51,10 +52,9 @@ class SideNavigationViewContainer extends Component {
{ href: '/setup', icon: 'setup', text: t('hub_side_navigation_setup') },
{ href: '/tutorial', icon: 'tutorial', text: t('hub_side_navigation_tutorial') },
{ href: '/plus', icon: 'plus', text: t('hub_side_navigation_supporter') },
{ href: '/rewards', icon: 'rewards', text: t('hub_side_navigation_rewards') },
...(IS_CLIQZ ? [] : [{ href: '/rewards', icon: 'rewards', text: t('hub_side_navigation_rewards') }]),
{ 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 },
{ id: 'logout', text: t('hub_side_navigation_log_out'), clickHandler: this._handleLogoutClick },
ProTip! Use n and p to navigate between commits in a pull request.