Skip to content
This repository was archived by the owner on Feb 5, 2025. It is now read-only.

Conversation

@yaelirub
Copy link
Contributor

@yaelirub yaelirub commented Feb 4, 2021

After talking this over with design (@mattmiklic ) we decided to remove this screen:
Simulator Screen Shot - iPhone 12 Pro - 2021-02-02 at 13 00 39

Instead, we will be sending the email immediately after the user entered their email :

RPReplay_Final1612412851.MP4

To test:

  1. copy this branch to pod file in WPiOS
  2. pod install
  3. Test login and signup work as expected with no issues
  4. Check tracking is as expected

@yaelirub yaelirub added the enhancement New feature or request label Feb 4, 2021
@yaelirub
Copy link
Contributor Author

yaelirub commented Feb 4, 2021

@mattmiklic , do you think we should update the copy on the first screen to let them know it has to be on the device?

@mattmiklic
Copy link
Member

I don't think we need to add any messaging to the Get Started screen, since at that point they could intend to log in via password and the email note wouldn't be relevant. The email confirmation is clear enough that it should be checked on the device, I think.

I do think it'd be good to add a message to the email confirmation screen letting them know they can go back if they didn't intend to sign up. Something like "Didn't mean to create a new account? Go back to re-enter your email address."

@yaelirub
Copy link
Contributor Author

yaelirub commented Feb 4, 2021

"Didn't mean to create a new account? Go back to re-enter your email address."

Love it! I'll add it. Thank you so much, @mattmiklic !

@ScoutHarris
Copy link
Contributor

I assume this isn't done yet since the "Didn't mean to create a new account?" message hasn't been added. So I'll just leave a comment that the podspec version needs to be bumped.

Copy link
Contributor

@ctarda ctarda left a comment

Choose a reason for hiding this comment

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

I did a quick test with WooCommerce, and even though this breaks the API, it does not remove any functionality that we might be using at the moment.

That being said, I would wait to merge, if possible, until @Garance91540 gives the 👍

@yaelirub yaelirub requested a review from emilylaguna February 4, 2021 20:58
@yaelirub
Copy link
Contributor Author

yaelirub commented Feb 4, 2021

Added copy:
IMG_7863C43CB5A6-1
@ScoutHarris , @mattmiklic , ready for another look

@mattmiklic
Copy link
Member

Looking good to me. 👍

@ScoutHarris ScoutHarris self-requested a review February 4, 2021 22:27
Copy link
Contributor

@ScoutHarris ScoutHarris left a comment

Choose a reason for hiding this comment

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

LGTM. After the version is corrected, :shipit: .

Copy link
Contributor

@emilylaguna emilylaguna left a comment

Choose a reason for hiding this comment

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

Tested this on:

  • iPhone Simulator - iOS 13
  • iPhone Simulator - iOS 14
  • iPad Simulator - iOS 14
  • Real Device: iPhone 11 Pro

Had no issues other than the unrelated login issue

LGTM 🚀 !

@yaelirub yaelirub force-pushed the task/send_email_after_email_entered branch from 86218ad to 77c6c9e Compare February 8, 2021 19:07
@yaelirub yaelirub requested a review from jkmassel February 8, 2021 19:19
Copy link
Contributor

@jkmassel jkmassel left a comment

Choose a reason for hiding this comment

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

Version change LGTM – I presume all the other files have no changed.

:shipit:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants