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.

Sign up for GitHub

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

SignupForm: remove legacy code for handling back button #91956

Merged
merged 1 commit into from
Jul 8, 2024
Merged

SignupForm: remove legacy code for handling back button #91956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 5 additions & 23 deletions client/blocks/signup-form/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import config from '@automattic/calypso-config';
import page from '@automattic/calypso-router';
import { Button, FormInputValidation, FormLabel } from '@automattic/components';
import { FormInputValidation, FormLabel } from '@automattic/components';
import { localizeUrl } from '@automattic/i18n-utils';
import { Spinner } from '@wordpress/components';
import clsx from 'clsx';
Expand Down Expand Up @@ -1028,16 +1028,6 @@ class SignupForm extends Component {
};

formFooter() {
if ( this.userCreationComplete() ) {
return (
<LoggedOutFormFooter>
<Button primary onClick={ () => this.props.goToNextStep() }>
{ this.props.translate( 'Continue' ) }
</Button>
</LoggedOutFormFooter>
);
}

const params = new URLSearchParams( window.location.search );
const variationName = params.get( 'variationName' );

Expand Down Expand Up @@ -1117,10 +1107,6 @@ class SignupForm extends Component {
);
}

userCreationComplete() {
return this.props.step && 'completed' === this.props.step.status;
}

handleOnChangeAccount = () => {
recordTracksEvent( 'calypso_signup_click_on_change_account' );
this.props.redirectToLogout( window.location.href );
Expand Down Expand Up @@ -1183,7 +1169,7 @@ class SignupForm extends Component {

{ this.renderWooCommerce() }

{ this.props.isSocialSignupEnabled && ! this.userCreationComplete() && (
{ this.props.isSocialSignupEnabled && (
<SocialSignupForm
handleResponse={ this.handleWooCommerceSocialConnect }
socialService={ this.props.socialService }
Expand Down Expand Up @@ -1230,7 +1216,6 @@ class SignupForm extends Component {
if ( this.props.isSocialFirst ) {
return (
<SignupFormSocialFirst
step={ this.props.step }
stepName={ this.props.stepName }
flowName={ this.props.flowName }
goToNextStep={ this.props.goToNextStep }
Expand All @@ -1251,8 +1236,7 @@ class SignupForm extends Component {
const isGravatar = this.props.isGravatar;
const emailErrorMessage = this.getErrorMessagesWithLogin( 'email' );
const showSeparator =
( ! config.isEnabled( 'desktop' ) && this.isHorizontal() && ! this.userCreationComplete() ) ||
this.props.isWoo;
( ! config.isEnabled( 'desktop' ) && this.isHorizontal() ) || this.props.isWoo;

if (
( this.props.isPasswordless &&
Expand Down Expand Up @@ -1287,7 +1271,6 @@ class SignupForm extends Component {
>
{ this.getNotice() }
<PasswordlessSignupForm
step={ this.props.step }
stepName={ this.props.stepName }
flowName={ this.props.flowName }
goToNextStep={ this.props.goToNextStep }
Expand Down Expand Up @@ -1324,8 +1307,7 @@ class SignupForm extends Component {
{ ! isGravatar && (
<>
{ showSeparator && <FormDivider /> }

{ this.props.isSocialSignupEnabled && ! this.userCreationComplete() && (
{ this.props.isSocialSignupEnabled && (
<SocialSignupForm
handleResponse={ this.props.handleSocialResponse }
socialService={ this.props.socialService }
Expand Down Expand Up @@ -1362,7 +1344,7 @@ class SignupForm extends Component {

{ showSeparator && <FormDivider /> }

{ this.props.isSocialSignupEnabled && ! this.userCreationComplete() && (
{ this.props.isSocialSignupEnabled && (
<SocialSignupForm
handleResponse={ this.props.handleSocialResponse }
socialService={ this.props.socialService }
Expand Down
13 changes: 0 additions & 13 deletions client/blocks/signup-form/passwordless.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,21 +269,8 @@ class PasswordlessSignupForm extends Component {
);
}

userCreationComplete() {
return this.props.step && 'completed' === this.props.step.status;
}

formFooter() {
const { isSubmitting } = this.state;
if ( this.userCreationComplete() ) {
return (
<LoggedOutFormFooter>
<Button primary onClick={ () => this.props.goToNextStep() }>
{ this.props.translate( 'Continue' ) }
</Button>
</LoggedOutFormFooter>
);
}
const submitButtonText = isSubmitting
? this.props.submitButtonLoadingLabel || this.props.translate( 'Creating Your Account…' )
: this.props.submitButtonLabel || this.props.translate( 'Create your account' );
Expand Down
3 changes: 0 additions & 3 deletions client/blocks/signup-form/signup-form-social-first.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import './style.scss';

interface SignupFormSocialFirst {
goToNextStep: () => void;
step: object;
stepName: string;
flowName: string;
redirectToAfterLoginUrl: string;
Expand Down Expand Up @@ -60,7 +59,6 @@ const options = {

const SignupFormSocialFirst = ( {
goToNextStep,
step,
stepName,
flowName,
redirectToAfterLoginUrl,
Expand Down Expand Up @@ -159,7 +157,6 @@ const SignupFormSocialFirst = ( {
return (
<div className="signup-form-social-first-email">
<PasswordlessSignupForm
step={ step }
stepName={ stepName }
flowName={ flowName }
goToNextStep={ goToNextStep }
Expand Down
1 change: 0 additions & 1 deletion client/landing/stepper/declarative-flow/internals/steps-repository/user/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ const StepContent: Step = ( { flow, stepName, navigation } ) => {
<>
<FormattedHeader align="center" headerText={ translate( 'Create your account' ) } brandFont />
<SignupFormSocialFirst
step={ {} }
stepName={ stepName }
flowName={ flow }
goToNextStep={ () => {
Expand Down
9 changes: 2 additions & 7 deletions client/signup/config/flows.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
import { isOnboardingGuidedFlow, isSiteAssemblerFlow } from '@automattic/onboarding';
import { get, includes, reject } from 'lodash';
import { getPlanCartItem } from 'calypso/lib/cart-values/cart-items';
import detectHistoryNavigation from 'calypso/lib/detect-history-navigation';
import { getQueryArgs } from 'calypso/lib/query-args';
import { addQueryArgs } from 'calypso/lib/url';
import { generateFlows } from 'calypso/signup/config/flows-pure';
Expand Down Expand Up @@ -386,14 +385,10 @@ const Flows = {
}

if ( isUserLoggedIn ) {
const urlParams = new URLSearchParams( window.location.search );
const param = urlParams.get( 'user_completed' );
const isUserStepOnly = flow.steps.length === 1 && stepConfig[ flow.steps[ 0 ] ].providesToken;

// Remove the user step unless the user has just completed the step
// and then clicked the back button.
// If the user step is the only step in the whole flow, e.g. /start/account, don't remove it as well.
if ( ! param && ! detectHistoryNavigation.loadedViaHistory() && ! isUserStepOnly ) {
// Remove the user step unless it is the only step in the whole flow, e.g., `/start/account`
if ( ! isUserStepOnly ) {
flow = removeUserStepFromFlow( flow );
}
}
Expand Down
36 changes: 9 additions & 27 deletions client/signup/steps/user/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import SignupForm from 'calypso/blocks/signup-form';
import JetpackLogo from 'calypso/components/jetpack-logo';
import WooCommerceConnectCartHeader from 'calypso/components/woocommerce-connect-cart-header';
import { initGoogleRecaptcha, recordGoogleRecaptchaAction } from 'calypso/lib/analytics/recaptcha';
import detectHistoryNavigation from 'calypso/lib/detect-history-navigation';
import { getSocialServiceFromClientId } from 'calypso/lib/login';
import {
isA4AOAuth2Client,
Expand Down Expand Up @@ -146,21 +145,12 @@ export class UserStep extends Component {
recaptchaClientId: null,
};

componentDidUpdate() {
if ( this.props.step?.status === 'completed' ) {
componentDidUpdate( prevProps ) {
if (
prevProps.step?.status !== this.props.step?.status &&
this.props.step?.status === 'completed'
) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change fixes a bug where the navigation from /start/user to the next step (start/domain) would be triggered twice, because the component happens to be rerendered twice in the completed state. Then you end up with /start/domain being twice in the window history, and you'll be confused when clicking the back button. You have to click twice.

This updated code is equivalent to how useEffect would work:

useEffect( () => {
  if ( status === 'completed' ) goToNextStep();
}, [ completed ] );

this.props.goToNextStep();
return;
}

if ( this.userCreationCompletedAndHasHistory( this.props ) ) {
// It looks like the user just completed the User Registartion Step
// And clicked the back button. Lets redirect them to the this page but this time they will be logged in.
const url = new URL( window.location );
const searchParams = url.searchParams;
searchParams.set( 'user_completed', true );
url.search = searchParams.toString();
// Redirect to itself and append ?user_completed
window.location.replace( url.toString() );
}
}

Expand Down Expand Up @@ -473,10 +463,6 @@ export class UserStep extends Component {
return this.props.step && 'completed' === this.props.step.status;
}

userCreationCompletedAndHasHistory( props ) {
return 'completed' === props.step?.status && detectHistoryNavigation.loadedViaHistory();
}

userCreationPending() {
return this.props.step && 'pending' === this.props.step.status;
}
Expand Down Expand Up @@ -583,10 +569,6 @@ export class UserStep extends Component {
return translate( 'Creating Your Account…' );
}

if ( this.userCreationComplete() ) {
return translate( 'Account created - Go to next step' );
}

return translate( 'Create your account' );
}

Expand Down Expand Up @@ -736,6 +718,10 @@ export class UserStep extends Component {
}

render() {
if ( this.userCreationComplete() ) {
return null; // return nothing so that we don't see the completed signup form flash.
}

if ( isP2Flow( this.props.flowName ) ) {
return this.renderP2SignupStep();
}
Expand All @@ -748,10 +734,6 @@ export class UserStep extends Component {
return this.renderGravatarSignupStep();
}

if ( this.userCreationCompletedAndHasHistory( this.props ) ) {
return null; // return nothing so that we don't see the error message and the sign up form.
}

// TODO: decouple hideBack flag from the flow name.
return (
<StepWrapper
Expand Down
Loading