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

SignupProcessingScreen refinements (Fix to #6114) #6284

Merged
merged 4 commits into from
Jun 24, 2016
Merged

Conversation

coreh
Copy link
Contributor

@coreh coreh commented Jun 23, 2016

This PR fixes Issue #6114, by making it so that the SignupFlowController is reset before the user clicks on the Continue button.

It also adds two other improvements:

  • The SignupProcessingScreenI18n stub is removed (We no longer need that, since the actual component landed)
  • The Sign Up link in the masterbar is no longer displayed during signup. (So that user's can't navigate to /signup by accidentally clicking on it)

Test live: https://calypso.live/?branch=fix/issue-6114

@coreh coreh added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Feature] Signup & Account Creation All screens and flows for making a new WordPress.com account. NUX labels Jun 23, 2016
@@ -13,7 +13,7 @@ import { localize } from 'i18n-calypso';
import Item from './item';
import config from 'config';

const MasterbarLoggedOut = ( { title, translate } ) => (
const MasterbarLoggedOut = ( { title, translate, showSignup = true } ) => (
Copy link
Member

Choose a reason for hiding this comment

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

Let's add showSignup to .propTypes in L44, and although I know they achieve the same thing, we should probably either define the default in defaultProps (L48 - like title), and remove it here, or define title = '' here, and remove .defaultProps for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I missed that entirely. Adding it.

@michaeldcain
Copy link
Member

@coreh: Just a small edit, but otherwise is looks and works great. Thanks for the quick fix!

@coreh
Copy link
Contributor Author

coreh commented Jun 24, 2016

Fixed. I also moved translate to the end of the props object destructuring, just for organization's sake, since it seems to be added by the localize function, at the bottom.

@michaeldcain
Copy link
Member

LGTM

@coreh coreh merged commit 2310ed6 into master Jun 24, 2016
@coreh coreh deleted the fix/issue-6114 branch June 24, 2016 19:19
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Signup & Account Creation All screens and flows for making a new WordPress.com account. NUX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants