Allow One-tap to be loaded on all pages. #9964

tofumatt · 2025-01-03T03:24:04Z

Summary

Addresses issue:

Enable One-Tap throughout site when SiwG One-Tap is enabled #9861

PR Author Checklist

My code is tested and passes existing unit tests.
My code has an appropriate set of unit tests which all pass.
My code is backward-compatible with WordPress 5.2 and PHP 7.4.
My code follows the WordPress coding standards.
My code has proper inline documentation.
I have added a QA Brief on the issue linked above.
I have signed the Contributor License Agreement (see https://cla.developers.google.com/).

Do not alter or remove anything below. The following sections will be managed by moderators only.

Code Reviewer Checklist

Run the code.
Ensure the acceptance criteria are satisfied.
Reassess the implementation with the IB.
Ensure no unrelated changes are included.
Ensure CI checks pass.
Check Storybook where applicable.
Ensure there is a QA Brief.
Ensure there are no unexpected significant changes to file sizes.

Merge Reviewer Checklist

Ensure the PR has the correct target branch.
Double-check that the PR is okay to be merged.
Ensure the corresponding issue has a ZenHub release assigned.
Add a changelog message to the issue.

github-actions · 2025-01-03T03:30:52Z

Build files for 4712b9c have been deleted.

github-actions · 2025-01-03T03:31:52Z

Size Change: +231 B (+0.01%)

Total Size: 1.98 MB

Filename	Size	Change
`./dist/assets/css/googlesitekit-admin-css-********************.min.css`	62.2 kB	+8 B (+0.01%)
`./dist/assets/js/googlesitekit-activation-********************.js`	24 kB	-2 B (-0.01%)
`./dist/assets/js/googlesitekit-entity-dashboard-********************.js`	81.4 kB	-1 B (0%)
`./dist/assets/js/googlesitekit-main-dashboard-********************.js`	166 kB	-6 B (0%)
`./dist/assets/js/googlesitekit-modules-ads-********************.js`	35.8 kB	-1 B (0%)
`./dist/assets/js/googlesitekit-modules-adsense-********************.js`	119 kB	-7 B (-0.01%)
`./dist/assets/js/googlesitekit-modules-analytics-4-********************.js`	191 kB	+5 B (0%)
`./dist/assets/js/googlesitekit-modules-pagespeed-insights-********************.js`	22.6 kB	+1 B (0%)
`./dist/assets/js/googlesitekit-modules-reader-revenue-manager-********************.js`	43.2 kB	+1 B (0%)
`./dist/assets/js/googlesitekit-modules-search-console-********************.js`	69.1 kB	+2 B (0%)
`./dist/assets/js/googlesitekit-modules-sign-in-with-google-********************.js`	31.2 kB	+243 B (+0.78%)
`./dist/assets/js/googlesitekit-modules-tagmanager-********************.js`	32.4 kB	-2 B (-0.01%)
`./dist/assets/js/googlesitekit-settings-********************.js`	127 kB	-7 B (-0.01%)
`./dist/assets/js/googlesitekit-splash-********************.js`	68.7 kB	-1 B (0%)
`./dist/assets/js/googlesitekit-vendor-********************.js`	322 kB	+1 B (0%)
`./dist/assets/js/googlesitekit-widgets-********************.js`	102 kB	-4 B (0%)
`./dist/assets/js/googlesitekit-wp-dashboard-********************.js`	63.2 kB	+1 B (0%)

ℹ️ View Unchanged

Filename	Size
`./dist/assets/css/googlesitekit-adminbar-css-********************.min.css`	11.8 kB
`./dist/assets/css/googlesitekit-authorize-application-css-********************.min.css`	846 B
`./dist/assets/css/googlesitekit-wp-dashboard-css-********************.min.css`	8.44 kB
`./dist/assets/js/33-********************.js`	2.76 kB
`./dist/assets/js/34-********************.js`	2.25 kB
`./dist/assets/js/35-********************.js`	3.64 kB
`./dist/assets/js/36-********************.js`	936 B
`./dist/assets/js/37-********************.js`	892 B
`./dist/assets/js/38-********************.js`	1.61 kB
`./dist/assets/js/39-********************.js`	1.57 kB
`./dist/assets/js/40-********************.js`	1.61 kB
`./dist/assets/js/41-********************.js`	1.59 kB
`./dist/assets/js/42-********************.js`	1.83 kB
`./dist/assets/js/43-********************.js`	3.12 kB
`./dist/assets/js/analytics-advanced-tracking-********************.js`	901 B
`./dist/assets/js/googlesitekit-ad-blocking-recovery-********************.js`	54.1 kB
`./dist/assets/js/googlesitekit-adminbar-********************.js`	34.8 kB
`./dist/assets/js/googlesitekit-api-********************.js`	10.1 kB
`./dist/assets/js/googlesitekit-components-gm2-********************.js`	6.17 kB
`./dist/assets/js/googlesitekit-components-gm3-********************.js`	10.1 kB
`./dist/assets/js/googlesitekit-consent-mode-********************.js`	25.6 kB
`./dist/assets/js/googlesitekit-data-********************.js`	2.37 kB
`./dist/assets/js/googlesitekit-datastore-forms-********************.js`	8.97 kB
`./dist/assets/js/googlesitekit-datastore-location-********************.js`	2.09 kB
`./dist/assets/js/googlesitekit-datastore-site-********************.js`	20.2 kB
`./dist/assets/js/googlesitekit-datastore-ui-********************.js`	10 kB
`./dist/assets/js/googlesitekit-datastore-user-********************.js`	28.2 kB
`./dist/assets/js/googlesitekit-events-provider-contact-form-7-********************.js`	646 B
`./dist/assets/js/googlesitekit-events-provider-easy-digital-downloads-********************.js`	624 B
`./dist/assets/js/googlesitekit-events-provider-mailchimp-********************.js`	630 B
`./dist/assets/js/googlesitekit-events-provider-ninja-forms-********************.js`	712 B
`./dist/assets/js/googlesitekit-events-provider-optin-monster-********************.js`	675 B
`./dist/assets/js/googlesitekit-events-provider-popup-maker-********************.js`	634 B
`./dist/assets/js/googlesitekit-events-provider-woocommerce-********************.js`	657 B
`./dist/assets/js/googlesitekit-events-provider-wpforms-********************.js`	633 B
`./dist/assets/js/googlesitekit-i18n-********************.js`	3.93 kB
`./dist/assets/js/googlesitekit-metric-selection-********************.js`	52.1 kB
`./dist/assets/js/googlesitekit-modules-********************.js`	22.4 kB
`./dist/assets/js/googlesitekit-notifications-********************.js`	37 kB
`./dist/assets/js/googlesitekit-polyfills-********************.js`	378 B
`./dist/assets/js/googlesitekit-user-input-********************.js`	43.9 kB
`./dist/assets/js/runtime-********************.js`	1.4 kB

_{compressed-size-action}

zutigrm

Thanks @tofumatt looks good, just 2 small suggestions

includes/Modules/Sign_In_With_Google.php

assets/js/modules/sign-in-with-google/components/common/OneTapToggles.js

zutigrm

LGTM, I skipped the suggestion I made initially since it will be proposed as part of another issue

eugene-manuilov

LGTM

eugene-manuilov · 2025-01-03T20:52:49Z

includes/Modules/Sign_In_With_Google.php

 		add_action( 'show_user_profile', $this->get_method_proxy( 'render_disconnect_profile' ) ); // This action shows the disconnect section on the users own profile page.
 		add_action( 'edit_user_profile', $this->get_method_proxy( 'render_disconnect_profile' ) ); // This action shows the disconnect section on other users profile page to allow admins to disconnect others.

-		add_action( 'woocommerce_login_form_start', $this->get_method_proxy( 'render_signin_button' ) );
+		// (Potentially) render the Sign in with Google button on all pages.
+		add_action( 'wp_footer', $this->get_method_proxy( 'render_signin_button' ) );


Actually, i am not sure that this is a good approach to use this version of the sign-in button script on all pages, especially on those that don't need the sign-in button. We should better revert changes in this function and create a new method that renders just one-tap script.

Plus, there is no wp_footer action call on the login page because it uses login_footer instead: https://github.com/WordPress/WordPress/blob/master/wp-login.php#L448

Oh, shoot, good point.

Yeah, I wondered about rendering only the script, but knowing in the future we want to actually be able to render the sign-in buttons on any page and the code already checks if it's on the login pages, maybe it'd be better for now to restrict the button transformations to login pages but always render the script?

We could change the method name to something like load_siwg_script or something instead 🤔

The code already only does button replacements on the relevant pages:

site-kit-wp/includes/Modules/Sign_In_With_Google.php

Lines 346 to 352 in 7ac6818

document.getElementsByClassName( 'login' )[0]?.insertBefore( parent, document.getElementsByClassName( 'woocommerce-form-row' )[0] );

<?php endif; // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>

async function handleCredentialResponse( response ) {

try {

const res = await fetch( '<?php echo esc_js( $login_uri ); ?>', {

method: 'POST',

I think that's fair for now, given in the future we'll want to allow the buttons to replaced on all pages, so rather than separate them out only to combine them again later, we can leave them as-is.

Ok, but let's not define a new div and render the button if it is neither the login page nor a woocommerce login form. Also, there is no sense in adding the redirect_to part as well because one-tap will never have that parameter on random pages.

I mean that we need to skip the following blocks if it is not a login page/form:

site-kit-wp/includes/Modules/Sign_In_With_Google.php

Lines 330 to 335 in 63f3394

const parent = document.createElement( 'div' );

<?php if ( $is_woo_commerce_login ) : // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>

document.getElementsByClassName( 'login' )[0]?.insertBefore( parent, document.getElementsByClassName( 'woocommerce-form-row' )[0] );

<?php else : // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>

document.getElementById( 'login' ).insertBefore( parent, document.getElementById( 'loginform' ) );

<?php endif; // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>

site-kit-wp/includes/Modules/Sign_In_With_Google.php

Line 358 in 63f3394

google.accounts.id.renderButton( parent, <?php echo wp_json_encode( $btn_args ); ?> );

site-kit-wp/includes/Modules/Sign_In_With_Google.php

Lines 364 to 368 in 63f3394

<?php if ( ! empty( $redirect_to ) ) : // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>

const expires = new Date();

expires.setTime( expires.getTime() + 1000 * 60 * 5 );

document.cookie = "<?php echo esc_js( Authenticator::COOKIE_REDIRECT_TO ); ?>=<?php echo esc_js( $redirect_to ); ?>;expires=" + expires.toUTCString() + ";path=<?php echo esc_js( Authenticator::get_cookie_path() ); ?>";

<?php endif; // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>

That makes sense 👍🏻

I've made the change 🙂

tofumatt · 2025-01-07T22:50:39Z

(JS Tests failing is not related to my changes)

eugene-manuilov

Thanks. LGTM.

Allow One-tap to be loaded on all pages.

7ac6818

zutigrm requested changes Jan 3, 2025

View reviewed changes

includes/Modules/Sign_In_With_Google.php Show resolved Hide resolved

assets/js/modules/sign-in-with-google/components/common/OneTapToggles.js Show resolved Hide resolved

zutigrm approved these changes Jan 3, 2025

View reviewed changes

eugene-manuilov approved these changes Jan 3, 2025

View reviewed changes

eugene-manuilov requested changes Jan 3, 2025

View reviewed changes

Update function name for Sign in with Google.

28135a8

tofumatt changed the base branch from develop to main January 7, 2025 15:51

tofumatt added 2 commits January 7, 2025 17:03

Rename test.

8beb1a2

Only render button code when appropriate.

4712b9c

eugene-manuilov approved these changes Jan 8, 2025

View reviewed changes

eugene-manuilov merged commit cb2d8c0 into main Jan 8, 2025
23 of 27 checks passed

eugene-manuilov deleted the siwg-one-tap-9861 branch January 8, 2025 11:19

tofumatt mentioned this pull request Jan 14, 2025

When using Sign in with Google (mainly One-Tap), users should be redirected back to the page they were on before signing in with Google. #10015

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow One-tap to be loaded on all pages. #9964

Allow One-tap to be loaded on all pages. #9964

tofumatt commented Jan 3, 2025

github-actions bot commented Jan 3, 2025 •

edited

Loading

github-actions bot commented Jan 3, 2025 •

edited

Loading

zutigrm left a comment

zutigrm left a comment

eugene-manuilov left a comment

eugene-manuilov Jan 3, 2025

tofumatt Jan 6, 2025

tofumatt Jan 6, 2025

eugene-manuilov Jan 7, 2025

eugene-manuilov Jan 7, 2025 •

edited

Loading

tofumatt Jan 7, 2025

tofumatt commented Jan 7, 2025

eugene-manuilov left a comment

	document.getElementsByClassName( 'login' )[0]?.insertBefore( parent, document.getElementsByClassName( 'woocommerce-form-row' )[0] );
	<?php endif; // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>

	async function handleCredentialResponse( response ) {
	try {
	const res = await fetch( '<?php echo esc_js( $login_uri ); ?>', {
	method: 'POST',

	const parent = document.createElement( 'div' );
	<?php if ( $is_woo_commerce_login ) : // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>
	document.getElementsByClassName( 'login' )[0]?.insertBefore( parent, document.getElementsByClassName( 'woocommerce-form-row' )[0] );
	<?php else : // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>
	document.getElementById( 'login' ).insertBefore( parent, document.getElementById( 'loginform' ) );
	<?php endif; // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>

	<?php if ( ! empty( $redirect_to ) ) : // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>
	const expires = new Date();
	expires.setTime( expires.getTime() + 1000 * 60 * 5 );
	document.cookie = "<?php echo esc_js( Authenticator::COOKIE_REDIRECT_TO ); ?>=<?php echo esc_js( $redirect_to ); ?>;expires=" + expires.toUTCString() + ";path=<?php echo esc_js( Authenticator::get_cookie_path() ); ?>";
	<?php endif; // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>

Allow One-tap to be loaded on all pages. #9964

Allow One-tap to be loaded on all pages. #9964

Conversation

tofumatt commented Jan 3, 2025

Summary

PR Author Checklist

Code Reviewer Checklist

Merge Reviewer Checklist

github-actions bot commented Jan 3, 2025 • edited Loading

github-actions bot commented Jan 3, 2025 • edited Loading

zutigrm left a comment

Choose a reason for hiding this comment

zutigrm left a comment

Choose a reason for hiding this comment

eugene-manuilov left a comment

Choose a reason for hiding this comment

eugene-manuilov Jan 3, 2025

Choose a reason for hiding this comment

tofumatt Jan 6, 2025

Choose a reason for hiding this comment

tofumatt Jan 6, 2025

Choose a reason for hiding this comment

eugene-manuilov Jan 7, 2025

Choose a reason for hiding this comment

eugene-manuilov Jan 7, 2025 • edited Loading

Choose a reason for hiding this comment

tofumatt Jan 7, 2025

Choose a reason for hiding this comment

tofumatt commented Jan 7, 2025

eugene-manuilov left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 3, 2025 •

edited

Loading

github-actions bot commented Jan 3, 2025 •

edited

Loading

eugene-manuilov Jan 7, 2025 •

edited

Loading