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-2032 Style Forgot Password in Hub #535

Merged
merged 10 commits into from Apr 28, 2020
Next
Style ForgotPassword Hub and rename locale variable to place
  • Loading branch information
benstrumeyer committed Apr 27, 2020
commit ca8d50552f991168e7358812c194bf17e79e4718
@@ -46,7 +46,7 @@ const Hub = () => (
<Route exact path="/rewards" component={RewardsView} />
<Route exact path="/products" component={ProductsView} />
<Route exact path="/create-account" component={CreateAccountView} />
<Route exact path="/forgot-password" render={() => <ForgotPasswordView locale="hub" />} />
<Route exact path="/forgot-password" render={() => <ForgotPasswordView place="hub" />} />

This comment has been minimized.

@Eden12345

Eden12345 Apr 27, 2020
Contributor

Another option is to have a boolean prop called hub. Then, all you would need here is to write <ForgotPasswordView hub />, and for the same component in the extension panel you wouldn't have to pass any prop (which will lead to it being falsey). This will also make the logic in your ForgotPassword.jsx file a little more straightforward. (This is just a suggestion; there is a lot of disagreement online about whether or not this is a good practice, but I prefer it which is why we do it in Insights.)

This comment has been minimized.

@benstrumeyer

benstrumeyer Apr 27, 2020
Author Contributor

I like this idea of boolean props. Changed to hub, however kept the panel variable for better readability

<Route exact path="/log-in" component={LogInView} />
</AppView>
);
@@ -51,7 +51,7 @@ const Ghostery = () => (
<Route path="/subscribe/:loggedIn" component={Subscribe} />
<Route path="/login" component={Login} />
<Route path="/create-account" component={CreateAccount} />
<Route path="/forgot-password" render={() => <ForgotPassword locale="panel" />} />
<Route path="/forgot-password" render={() => <ForgotPassword place="panel" />} />
<Route path="/account-success" component={AccountSuccess} />
</Panel>
);
@@ -46,9 +46,9 @@ class ForgotPassword extends React.Component {
e.preventDefault();
this.setState({ loading: true }, () => {
const { email } = this.state;
const { locale } = this.props;
const isInPanel = locale === 'panel';
const isInHub = locale === 'hub';
const { place } = this.props;
const isInPanel = place === 'panel';
const isInHub = place === 'hub';

// validate the email and password
if (!validateEmail(email)) {
@@ -86,9 +86,9 @@ class ForgotPassword extends React.Component {
*/
render() {
const { email, loading, emailError } = this.state;
const { locale } = this.props;
const isInPanel = locale === 'panel';
const isInHub = locale === 'hub';
const { place } = this.props;
const isInPanel = place === 'panel';
const isInHub = place === 'hub';
const buttonClasses = ClassNames('button ghostery-button', { loading });

const ContainerClassNames = ClassNames('', {
@@ -39,9 +39,33 @@
width: 456px;
}
.ForgotPasswordButtonsContainer {
margin-top: 10px;
margin-left: 3px;
width: 456px;
}
input#ForgotPasswordMessage {
// Foundation Overrides
border: 1px solid #c8c7c2;
border-radius: 0;
box-shadow: none;

&:focus {
// Foundation Overrides
box-shadow: none;
border-color: #4a4a4a;
}
}
#forgot-password-cancel {
border-color: #930194;
color: #930194;
width: 150px;
}
.button.ghostery-button {

This comment has been minimized.

@Eden12345

Eden12345 Apr 27, 2020
Contributor

Is there no way to use existing Sass from the Hub to take care of this and avoid duplicating markdown? For example, refactoring Sass from individual form components into helper classes in the root Sass file for the Hub, so that you can assign those classes in your ForgotPassword.jsx file when you're in the Hub view?

This comment has been minimized.

@benstrumeyer

benstrumeyer Apr 28, 2020
Author Contributor

Perfect, it looks like we do have the styling for these buttons already. I noticed button widths aren't standardized inside the hub, so we're still going to have to use width here

background-color: #930194;
width: 150px;
}
.loader:after {
background: #930194;
}
}

/* FORGOT PASSWORD SHARED */
ProTip! Use n and p to navigate between commits in a pull request.