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

Conversation

@selanthiraiyan
Copy link
Contributor

@selanthiraiyan selanthiraiyan commented Jul 5, 2022

Related to woocommerce/woocommerce-ios#7212
Design ref: p1657014068195729/1656997606.750839-slack-C03L1NF1EA3

Description

This PR adds What is WordPress.com? button in the Email entry screen of "Continue with WordPress.com" flow.

As we cannot expect all users who install the app to be familiar with "WordPress.com" we are adding this button to teach users about "WordPress.com".

This is part of "Experiment 2: Add more information about the login process" in pe5sF9-6C-p2

Testing instructions

Check out the branch from woocommerce/woocommerce-ios#7213 and follow the testing instructions specified in the PR.

Screenshots

Before After

@selanthiraiyan selanthiraiyan marked this pull request as ready for review July 5, 2022 14:45
@jaclync jaclync self-assigned this Jul 5, 2022
Copy link
Contributor

@jaclync jaclync left a comment

Choose a reason for hiding this comment

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

Nice work @selanthiraiyan navigating through the authenticator codebase and coming up with the changes so quickly! 💯

One thing I'd like to confirm is the design: is there a reference for the design with the underscored link? I couldn't seem to find it in the design post pe5sF9-1U-p2


/// Tracked when the user clicks “What is WordPress.com?" button on the WordPress.com flow screen
///
case whatIsWpCom = "what_is_wordpress_com"
Copy link
Contributor

Choose a reason for hiding this comment

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

super nit: it'd be nice having a consistent naming convention

Suggested change
case whatIsWpCom = "what_is_wordpress_com"
case whatIsWPCom = "what_is_wordpress_com"

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be worth confirming with WPiOS about adding a new event to the analytics tracker, I'm not super sure if they have a workflow for analytics like whether they keep track of all the events. @pmusolino do you know who we can reach out for taking a look at this, after the recent team changes in WPiOS? (I saw that a bunch of the authors aren't with WPiOS anymore)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

super nit: it'd be nice having a consistent naming convention

Done in 48fbd17

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might be worth confirming with WPiOS about adding a new event to the analytics tracker, I'm not super sure if they have a workflow for analytics like whether they keep track of all the events. @pmusolino do you know who we can reach out for taking a look at this, after the recent team changes in WPiOS? (I saw that a bunch of the authors aren't with WPiOS anymore)

I haven't had success with building https://github.com/wordpress-mobile/WordPress-iOS app. I will try to build again and validate this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have raised a PR in WP iOS wordpress-mobile/WordPress-iOS#18996 to test this.

I have also validated that the changes related to the analytics tracker are non-breaking.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea I don't think the change affects WPiOS, but just wanted to see if they have a centralized doc to keep track of the events

@jaclync
Copy link
Contributor

jaclync commented Jul 6, 2022

Hey @selanthiraiyan do you know if we have the design confirmed? Just wanted to check first since it probably takes more time to release another pod version than making any design changes again 😅

If we don't have a design reference yet, I can ask in the design requests today and hopefully hear back tomorrow.

Nice work @selanthiraiyan navigating through the authenticator codebase and coming up with the changes so quickly! 💯

One thing I'd like to confirm is the design: is there a reference for the design with the underscored link? I couldn't seem to find it in the design post pe5sF9-1U-p2

@selanthiraiyan
Copy link
Contributor Author

@jaclync Oh, I forgot about the question about design. Sorry. 🙏

I confirmed with the design team in slack and got approval for this design.

You can check the thread here p1657014068195729/1656997606.750839-slack-C03L1NF1EA3

Copy link
Contributor

@jaclync jaclync left a comment

Choose a reason for hiding this comment

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

LGTM, nice work @selanthiraiyan ! :shipit:

Thanks for verifying the design, I added the reference to the PR description. There are so many conversations on various platforms these days 😅

Tested with large font sizes, and the new event is tracked in the console 👍

@selanthiraiyan selanthiraiyan merged commit 849029b into trunk Jul 7, 2022
@selanthiraiyan selanthiraiyan deleted the feature/what-is-wp-com-button branch July 7, 2022 09:04
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.

3 participants