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

Social Auth: Update TOS message to remove mention of creating an account on social login #67057

Merged
merged 8 commits into from Sep 8, 2022
Merged

Social Auth: Update TOS message to remove mention of creating an account on social login #67057

merged 8 commits into from Sep 8, 2022

Conversation

zaguiini
Copy link
Contributor

@zaguiini zaguiini commented Aug 26, 2022 •
edited

Proposed Changes

On the login page, we warn the user that we're creating them an account if it doesn't exist when they try to socially sign in:

But that's not true: we don't try to, and warn them that they should sign up instead (notice on top). This is misleading, so we need to update the TOS. Also, a legal representative asked to differentiate the link color from the text: p2y3YZ-5Oq-p2#comment-13737.

Testing Instructions

Check that the login and sign-up pages do not suggest that we're creating a new account in case social login fails. It also puts the ToS copy at the beginning and covers all three authentication methods.

Don't forget to run Calypso locally. Copy the link and open it in incognito mode.

/log-in /log-in/new
image image
/log-in/jetpack /log-in?from=woocommerce-onboarding
image image
/log-in?from=p2 /start/user
image image
Jetpack Connect WooCommerce connect Crowdsignal connect
image image image

@zaguiini zaguiini added the Login label Aug 26, 2022
@zaguiini zaguiini requested review from enejb and a team August 26, 2022 19:20
@zaguiini zaguiini self-assigned this Aug 26, 2022
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 26, 2022
@github-actions
Copy link

github-actions bot commented Aug 26, 2022 •
edited by matticbot

Calypso Live (direct link)
https://calypso.live?image=registry.a8c.com/calypso/app:build-41600
Jetpack Cloud live (direct link)
https://calypso.live?image=registry.a8c.com/calypso/app:build-41600&env=jetpack

@matticbot
Copy link
Contributor

matticbot commented Aug 26, 2022 •
edited

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~167 bytes removed 📉 [gzipped])

name         parsed_size           gzip_size
entry-login      -1066 B  (-0.1%)     -167 B  (-0.1%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~222 bytes removed 📉 [gzipped])

name             parsed_size           gzip_size
jetpack-connect      -1184 B  (-0.1%)     -222 B  (-0.1%)
accept-invite         -118 B  (-0.0%)      -68 B  (-0.1%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~231 bytes removed 📉 [gzipped])

name                          parsed_size           gzip_size
async-load-design-blocks          -1066 B  (-0.0%)     -163 B  (-0.0%)
async-load-signup-steps-user       -118 B  (-0.1%)      -68 B  (-0.1%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@zaguiini
Copy link
Contributor Author

Working with a representative in p2y3YZ-5Oq-p2#comment-13724.

@zaguiini zaguiini marked this pull request as draft August 29, 2022 17:24
@zaguiini zaguiini force-pushed the update/login-form-social-tos branch from cacefb6 to ce0d7ff Compare August 31, 2022 18:58
@zaguiini zaguiini marked this pull request as ready for review August 31, 2022 19:12
@zaguiini zaguiini force-pushed the update/login-form-social-tos branch from ce0d7ff to e5a5250 Compare August 31, 2022 20:35
@zaguiini zaguiini requested a review from a team August 31, 2022 20:35
@zaguiini zaguiini force-pushed the update/login-form-social-tos branch from e5a5250 to daf4c27 Compare August 31, 2022 20:50
zaguiini
zaguiini commented Aug 31, 2022
View reviewed changes
client/blocks/login/social.jsx Outdated
Comment on lines 143 to 208
renderSocialTos = () => {
const { redirectTo, translate } = this.props;

const isJetpackMagicLinkSignUpFlow =
redirectTo &&
redirectTo.includes( 'jetpack/connect' ) &&
config.isEnabled( 'jetpack/magic-link-signup' );

const tosLink = (
<a
href={ localizeUrl( 'https://wordpress.com/tos/' ) }
target="_blank"
rel="noopener noreferrer"
/>
);
const privacyLink = (
<a
href={ localizeUrl( 'https://automattic.com/privacy/' ) }
target="_blank"
rel="noopener noreferrer"
/>
);

if ( isJetpackMagicLinkSignUpFlow ) {
return (
<>
<p className="login__social-tos">
{ translate(
'By continuing, you agree to our {{tosLink}}Terms of' +
' Service{{/tosLink}} and acknowledge that you have read our' +
' {{privacyLink}}Privacy Policy{{/privacyLink}}.',
{
components: {
tosLink,
privacyLink,
},
}
) }
</p>
<p className="login__social-tos">
{ translate(
'If you continue with Google, Apple, or an email that isn’t registered yet,' +
' you are creating a new WordPress.com account.'
) }
</p>
</>
);
}
return (
<p className="login__social-tos">
{ translate(
'If you continue with Google or Apple,' +
' you agree to our' +
' {{tosLink}}Terms of Service{{/tosLink}}, and have' +
' read our {{privacyLink}}Privacy Policy{{/privacyLink}}.',
{
components: {
tosLink,
privacyLink,
},
}
) }
</p>
);
};

Copy link
Contributor Author

@zaguiini zaguiini Aug 31, 2022 •
edited

Choose a reason for hiding this comment

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

Now that we're showing a single ToS on the login page above the "Continue" button, there's no point in having this in the component as well.

It doesn't affect the sign-up page, which has its own social component called SocialSignupForm.

@danielbachhuber danielbachhuber added the [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging label Sep 1, 2022
@danielbachhuber
Copy link
Contributor

Added the [Status] String Freeze label so we can make sure the strings are translated before deploying.

danielbachhuber
danielbachhuber approved these changes Sep 1, 2022
View reviewed changes
Copy link
Contributor

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

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

Works great 👍

Nice work tracking down all of the login variations!

@a8ci18n
Copy link

a8ci18n commented Sep 1, 2022

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/7507490

Thank you @zaguiini for including a screenshot in the description! This is really helpful for our translators.

@enejb
Copy link
Member

enejb commented Sep 1, 2022 •
edited

Nice work with the changes @zaguiini.

I found one more login case
Screen Shot 2022-09-01 at 2 49 26 PM
Woo login

You get to it from woocommerace.com.

Which looks correct.

I found this page
Screen Shot 2022-09-01 at 2 54 11 PM

from woocommerce-onboarding to be quite broken but I am not sure how it is used.
But the broken things are not new in this PR :( - We should fix this in a different PR. (Not a blocker for this change)
it looks to me like the header shouldn't be there at all but it is. Also the styles seem really off.

enejb
enejb approved these changes Sep 1, 2022
View reviewed changes
Copy link
Member

@enejb enejb 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!

@zaguiini
Copy link
Contributor Author

zaguiini commented Sep 2, 2022

Thanks, @enejb! Asked this in the Woo channel on Slack: p1662149049437139-slack-C07418EA0

@a8ci18n
Copy link

a8ci18n commented Sep 8, 2022

Translation for this Pull Request has now been finished.

@danielbachhuber danielbachhuber merged commit d5966bf into trunk Sep 8, 2022
@danielbachhuber danielbachhuber deleted the update/login-form-social-tos branch September 8, 2022 12:29
@danielbachhuber danielbachhuber changed the title Social Auth: Do not say we are creating an account on social login Social Auth: Update TOS message to remove mention of creating an account on social login Sep 8, 2022
@github-actions github-actions bot removed [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielbachhuber danielbachhuber danielbachhuber approved these changes

@enejb enejb enejb approved these changes

Assignees

@zaguiini zaguiini

Labels
Login
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@zaguiini @matticbot @danielbachhuber @a8ci18n @enejb