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

Remove updates and promotions checkbox and dropdown #659

Merged
merged 3 commits into from Jan 19, 2021
+28 −108
Merged

Remove updates and promotions checkbox and dropdown #659

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7201ab2
Remove learn more dropdown and update design
benstrumeyer Jan 13, 2021
a8a7552
Remove updates and promotions checkbox and automatically subscribe th…
benstrumeyer Jan 13, 2021
f949dfa
Remove ,
benstrumeyer Jan 13, 2021
File filter
Filter file types
Clear filters
Jump to
Jump to file
Failed to load files.

Always

Just for now

Prev Next
Loading status checks…

Remove updates and promotions checkbox and automatically subscribe th… 

…e user to emails upon registering in the hub instead
  • Loading branch information
@benstrumeyer
benstrumeyer committed Jan 13, 2021
commit a8a755216d40ff034b03d8da8859e4b7edda9234
6 _locales/en/messages.json
View file
Open in desktop
@@ -1848,12 +1848,6 @@
"ghostery_browser_hub_onboarding_already_have_account": {
"message": "I already have an account."
},
"ghostery_browser_hub_onboarding_send_me": {
"message": "Send me"
},
"ghostery_browser_hub_onboarding_updates_and_promotions": {
"message": "updates & promotions."
},
"ghostery_browser_hub_onboarding_skip": {
"message": "Skip"
},
8 app/Account/AccountActions.js
View file
Open in desktop
@@ -28,7 +28,7 @@ import {
GET_USER_SETTINGS_FAIL,
GET_USER_SUBSCRIPTION_DATA_FAIL,
GET_USER_SUBSCRIPTION_DATA_SUCCESS,
ACCOUNT_DATA_EMAIL_PREFERENCES_CHECKBOX_CHANGE
SUBSCRIBE_TO_EMAIL_LIST
} from './AccountConstants';
import { SET_TOAST } from '../hub/Views/AppView/AppViewConstants';
import { CLEAR_THEME } from '../panel/constants/constants';
@@ -203,9 +203,9 @@ export const resetPassword = email => dispatch => (
})
);

export const handleEmailPreferencesCheckboxChange = (name, checked) => dispatch => (
export const subscribeToEmailList = name => dispatch => (
dispatch({
type: ACCOUNT_DATA_EMAIL_PREFERENCES_CHECKBOX_CHANGE,
payload: { name, checked },
This conversation was marked as resolved by wlycdgr
Comment on lines -206 to -209

This comment has been minimized.

Sign in to view
@wlycdgr

wlycdgr Jan 13, 2021
Member

No other parts of the code were using this action creator? Double checking because this AccountActions file is not specific to the new onboarding

This comment has been minimized.

Sign in to view
@benstrumeyer

benstrumeyer Jan 14, 2021
Author Contributor

Good double check! I don't believe so, I made this const/action/reducer specifically for that checkbox
type: SUBSCRIBE_TO_EMAIL_LIST,
payload: { name },
})
);
2 app/Account/AccountConstants.js
View file
Open in desktop
@@ -38,4 +38,4 @@ export const GET_USER_SUBSCRIPTION_DATA_FAIL = 'GET_USER_SUBSCRIPTION_DATA_FAIL'
export const GET_USER_SUBSCRIPTION_DATA_SUCCESS = 'GET_USER_SUBSCRIPTION_DATA_SUCCESS';

// Opt into/out of updates and promotions sendgrid emails
export const ACCOUNT_DATA_EMAIL_PREFERENCES_CHECKBOX_CHANGE = 'ACCOUNT_DATA_EMAIL_PREFERENCES_CHECKBOX_CHANGE';

This comment has been minimized.

Sign in to view
@wlycdgr

wlycdgr Jan 13, 2021
Member

Same deal, just double checking this isn't being used anywhere else anymore. If not, and the user can no longer change their email preferences in the extension panel or either of the hubs, we should be able to dispense with the SUBSCRIBE_TO_EMAIL_LIST action also, no?

This comment has been minimized.

Sign in to view
@benstrumeyer

benstrumeyer Jan 14, 2021
Author Contributor

We actually don't have the functionality in the original hub or the panel to toggle sendgrid emails on, so this is only used here
export const SUBSCRIBE_TO_EMAIL_LIST = 'SUBSCRIBE_TO_EMAIL_LIST';
8 app/Account/AccountReducer.js
View file
Open in desktop
@@ -21,7 +21,7 @@ import {
GET_USER_SUBSCRIPTION_DATA_SUCCESS,
RESET_PASSWORD_SUCCESS,
RESET_PASSWORD_FAIL,
ACCOUNT_DATA_EMAIL_PREFERENCES_CHECKBOX_CHANGE
SUBSCRIBE_TO_EMAIL_LIST
} from './AccountConstants';
import { UPDATE_PANEL_DATA } from '../panel/constants/constants';

@@ -119,11 +119,11 @@ export default (state = initialState, action) => {
resetPasswordError: true
};
}
case ACCOUNT_DATA_EMAIL_PREFERENCES_CHECKBOX_CHANGE: {
const { name, checked } = action.payload;
case SUBSCRIBE_TO_EMAIL_LIST: {
const { name } = action.payload;
let emailPreferences;
if (name === 'global') {
emailPreferences = { ...state.user.emailPreferences, ...{ global: checked } };
emailPreferences = { ...state.user.emailPreferences, ...{ global: true } };
This conversation was marked as resolved by wlycdgr
Comment on lines +122 to +126

This comment has been minimized.

Sign in to view
@wlycdgr

wlycdgr Jan 13, 2021
Member

As alluded to above, do we need this action at all given that notify_promotions and notify_upgrade_updates now default to true in ConfData?

This comment has been minimized.

Sign in to view
@benstrumeyer

benstrumeyer Jan 14, 2021
Author Contributor

I made this const to automatically subscribe people to sendgrid emails--the function is taken from a button in account-web that Teresa pointed me to. I believe notify_promotions and notify_upgrade_updates are different. notify-promotions are for displaying modals, and notify_upgrade_updates are for showing upgrade alerts when we push a new version. Can you double check me on this?

This comment has been minimized.

Sign in to view
@wlycdgr

wlycdgr Jan 14, 2021
Member

You are right about notify_promotions and notify_upgrade_updates! We still may not need this action though if we now subscribe the user to emails automatically on the account server on account creation.
}
const user = { ...state.user, ...{ emailPreferences } };
return { ...state, ...{ user } };
23 ...ery-browser-hub/Views/OnboardingViews/Step1_CreateAccountForm/Step1_CreateAccountForm.jsx
View file
Open in desktop
@@ -16,8 +16,6 @@ import PropTypes from 'prop-types';
import ClassNames from 'classnames';
import ToggleCheckbox from '../../../../shared-components/ToggleCheckbox';

const promoString = `${t('ghostery_browser_hub_onboarding_send_me')} Ghostery ${t('ghostery_browser_hub_onboarding_updates_and_promotions')}`;

/**
* A Functional React component for rendering the Browser Create Account View
* @return {JSX} JSX for rendering the Browser Create Account View of the Hub app
@@ -39,9 +37,7 @@ const Step1_CreateAccountForm = (props) => {
legalConsentChecked,
legalConsentNotCheckedError,
handleLegalConsentCheckboxChange,
isUpdatesChecked,
handleInputChange,
handleUpdatesCheckboxChange,
handleSubmit,
} = props;

@@ -192,23 +188,6 @@ const Step1_CreateAccountForm = (props) => {
)}
</div>
</div>
<div className="row align-center-middle">
<div className="columns small-10 medium-8">
<div className="Step1_CreateAccountForm__checkboxContainer Step1_CreateAccountForm--marginBottom flex-container">
<ToggleCheckbox
checked={isUpdatesChecked}
className="ToggleCheckbox--flush-left"
onChange={handleUpdatesCheckboxChange}
/>
<span
className="Step1_CreateAccountForm__promoString"
onClick={handleUpdatesCheckboxChange}
dangerouslySetInnerHTML={{ __html: promoString }}
/>
</div>
</div>
<div className="columns small-10 medium-2" />
</div>
<div className="row align-center-middle">
<div className="columns small-10 medium-8">
<div className="Step1_CreateAccountForm__checkboxContainer BrowserCreateAccountForm--marginBottom flex-container">
@@ -242,13 +221,11 @@ Step1_CreateAccountForm.propTypes = {
confirmEmailError: PropTypes.bool.isRequired,
firstName: PropTypes.string.isRequired,
lastName: PropTypes.string.isRequired,
isUpdatesChecked: PropTypes.bool.isRequired,
password: PropTypes.string.isRequired,
confirmPassword: PropTypes.string.isRequired,
passwordInvalidError: PropTypes.bool.isRequired,
passwordLengthError: PropTypes.bool.isRequired,
handleInputChange: PropTypes.func.isRequired,
handleUpdatesCheckboxChange: PropTypes.func.isRequired,
handleSubmit: PropTypes.func.isRequired,
};

7 ...ry-browser-hub/Views/OnboardingViews/Step1_CreateAccountForm/Step1_CreateAccountForm.scss
View file
Open in desktop
@@ -48,13 +48,6 @@
color: red;
}
}
.Step1_CreateAccountForm__promoString {
font-size: 14px;
margin-top: 15px;
@include breakpoint(small down) {
margin-top: 0;
}
}
.Step1_CreateAccountForm__legalConsentCheckedLabel {
font-size: 14px;
@include breakpoint(small down) {
11 ...er-hub/Views/OnboardingViews/Step1_CreateAccountForm/Step1_CreateAccountFormContainer.jsx
View file
Open in desktop
@@ -113,13 +113,6 @@ class CreateAccountFormContainer extends Component {
this.setState(prevState => ({ legalConsentChecked: !prevState.legalConsentChecked }));
}

/**
* Update updates checkbox value by updating state
*/
_handleUpdatesCheckboxChange = () => {
this.setState(prevState => ({ isUpdatesChecked: !prevState.isUpdatesChecked }));
}

/**
* Handle creating an account, but validate the data first.
* @param {Object} event the 'submit' event
@@ -134,7 +127,6 @@ class CreateAccountFormContainer extends Component {
legalConsentChecked,
password,
confirmPassword,
isUpdatesChecked,
} = this.state;
const emailIsValid = email && validateEmail(email);
const confirmIsValid = confirmEmail && validateConfirmEmail(email, confirmEmail);
@@ -164,7 +156,7 @@ class CreateAccountFormContainer extends Component {
if (success) {
// User is automatically logged in, and redirected to the logged in view of BrowserCreateAccountForm
actions.getUser().then(() => {
if (isUpdatesChecked) actions.handleEmailPreferencesCheckboxChange('global', isUpdatesChecked);
actions.subscribeToEmailList('global');

This comment has been minimized.

Sign in to view
@wlycdgr

wlycdgr Jan 13, 2021
Member

See above - we may not need this at all?

This comment has been minimized.

Sign in to view
@benstrumeyer

benstrumeyer Jan 14, 2021
Author Contributor

See comment above
});
// Toggle legal consent checked here
actions.setToast({
@@ -222,7 +214,6 @@ class CreateAccountFormContainer extends Component {
handleLegalConsentCheckboxChange={this._handleLegalConsentCheckboxChange}
handleSubmit={this._handleCreateAccountAttempt}
isUpdatesChecked={isUpdatesChecked}
handleUpdatesCheckboxChange={this._handleUpdatesCheckboxChange}
/>
);
}
4 app/ghostery-browser-hub/Views/OnboardingViews/Step1_CreateAccountForm/index.js
View file
Open in desktop
@@ -13,15 +13,15 @@

import { buildReduxHOC } from '../../../../shared-hub/utils';
import Step1_CreateAccountFormContainer from './Step1_CreateAccountFormContainer';
import { register, getUser, handleEmailPreferencesCheckboxChange } from '../../../../Account/AccountActions';
import { register, getUser, subscribeToEmailList } from '../../../../Account/AccountActions';
import { setToast } from '../../../../hub/Views/AppView/AppViewActions';

const stateSlices = ['account'];
const actionCreators = {
setToast,
register,
getUser,
handleEmailPreferencesCheckboxChange
subscribeToEmailList
};

export default buildReduxHOC(stateSlices, actionCreators, Step1_CreateAccountFormContainer);
4 app/hub/Views/CreateAccountView/index.js
View file
Open in desktop
@@ -15,7 +15,7 @@ import { connect } from 'react-redux';
import { bindActionCreators } from 'redux';

import CreateAccountViewContainer from './CreateAccountViewContainer';
import { register, getUser } from '../../../Account/AccountActions';
import { register, getUser, subscribeToEmailList } from '../../../Account/AccountActions';
import { setToast } from '../AppView/AppViewActions';

/**
@@ -36,7 +36,7 @@ const mapDispatchToProps = dispatch => ({
actions: bindActionCreators({
setToast,
register,
getUser
getUser,
}, dispatch),
});

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