Playground onboarding flow: update url via react router so next step can pick up playground GET param correctly by ashfame · Pull Request #102037 · Automattic/wp-calypso

ashfame · 2025-03-28T11:33:13Z

Related to #101948

Proposed Changes

update search params through react router for next step to pick GET param accurately

Testing Instructions

As a logged out user, head over to http://calypso.localhost:3000/setup/onboarding/playground & you will notice ?playground gets added to the URL
Click "Launch on WordPress.com"
You will get to the user screen http://calypso.localhost:3000/setup/onboarding/user?playground=xxx

Ensure playground GET param is not missing from url

Pre-merge Checklist

Has the general commit checklist been followed? (PCYsg-hS-p2)
Have you written new tests for your changes?
Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
Have you checked for TypeScript, React or other console errors?
Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
- For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

…aram correctly

github-actions · 2025-03-28T11:33:25Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-138852

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-138852&env=jetpack

Automattic for Agencies live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-138852&env=a8c-for-agencies

Copilot

Pull Request Overview

This PR updates the playground onboarding flow so that the playground GET parameter is accurately propagated via React Router for subsequent steps.

Added a new setSearchParams callback parameter to update URL search parameters through React Router.
Updated the initializeWordPressPlayground function to call setSearchParams and modified the PlaygroundIframe component to pass the new callback.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
client/landing/stepper/declarative-flow/internals/steps-repository/playground/lib/initialize-playground.ts	Added setSearchParams parameter and updated URL search params via both window.history.replaceState and the React Router method.
client/landing/stepper/declarative-flow/internals/steps-repository/playground/components/playground-iframe/index.tsx	Imported and passed setSearchParams from useSearchParams to initializeWordPressPlayground.

Comments suppressed due to low confidence (2)

client/landing/stepper/declarative-flow/internals/steps-repository/playground/lib/initialize-playground.ts:27

Mutating the existing URLSearchParams object may not trigger a re-render; consider returning a new URLSearchParams instance (e.g., new URLSearchParams(prev) with the updated value) to ensure React Router properly detects the change.

setSearchParams( ( prev ) => { prev.set( 'playground', playgroundId as string ); return prev; } );

client/landing/stepper/declarative-flow/internals/steps-repository/playground/lib/initialize-playground.ts:24

Using both window.history.replaceState and setSearchParams to update the URL may lead to redundant or conflicting state updates; consider consolidating URL updates to use React Router exclusively if possible.

window.history.replaceState( {}, '', url.toString() );

matticbot · 2025-03-28T11:41:22Z

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

bgrgicak · 2025-03-28T11:41:54Z

...eclarative-flow/internals/steps-repository/playground/components/playground-iframe/index.tsx

 } ) {
 	const iframeRef = useRef< HTMLIFrameElement >( null );
 	const recommendedPHPVersion = usePhpVersions().recommendedValue;
+	const [ , setSearchParams ] = useSearchParams();


Suggested change

const [ , setSearchParams ] = useSearchParams();

const [ setSearchParams ] = useSearchParams();

Don't think this is correct. We need to skip the first returned value and assign second returned value to the const. Can you explain?

Don't think this is correct. We need to skip the first returned value and assign second returned value to the const. Can you explain?

Yes, it's the current version. We only need the set function.

@zaerl Do you mean to say the current version is correct or the suggestion version is?

@zaerl Do you mean to say the current version is correct or the suggestion version is?

The current one is right.

bgrgicak · 2025-03-28T11:53:45Z

.../stepper/declarative-flow/internals/steps-repository/playground/lib/initialize-playground.ts

 		url.searchParams.set( 'playground', playgroundId );
 		window.history.replaceState( {}, '', url.toString() );


setSearchParams updates the navigation, so we don't need these lines anymore.

I actually removed them first but browser history state wasn't updated so had to add them back again. Know what might be going on?

No, I tried it locally and it worked for me, but if you have the issue let's leave it as-is.

I don't have experience with react-router-dom, but from reading their docs it sounded like the navigation should be updated.

@bgrgicak Just to confirm how I tested this: Without these lines, if I am going back by the browser's back button, I get to http://calypso.localhost:3000/setup/onboarding/playground?playground=9a566c25-a356-4b43-8438-ccfc6fc3cbf0 and then http://calypso.localhost:3000/setup/onboarding/playground so history state wasn't replaced.

matticbot · 2025-03-28T11:55:18Z

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

blaze-dashboard
help-center
notifications
wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug fix/playground-onboarding-flow/101948 on your sandbox.

zaerl

Working as expected. Good job here.

ashfame · 2025-03-28T12:38:50Z

@bgrgicak Gonna land this, but feel free to comment further and I can address in follow up PR, if needed.

update url via react router so next step can pick up playground GET p…

4bf0144

…aram correctly

ashfame requested review from bgrgicak, Copilot and zaerl March 28, 2025 11:33

ashfame self-assigned this Mar 28, 2025

matticbot added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 28, 2025

Copilot AI reviewed Mar 28, 2025

View reviewed changes

bgrgicak reviewed Mar 28, 2025

View reviewed changes

zaerl approved these changes Mar 28, 2025

View reviewed changes

ashfame merged commit 79dbd3e into trunk Mar 28, 2025
18 checks passed

ashfame deleted the fix/playground-onboarding-flow/101948 branch March 28, 2025 12:39

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 28, 2025

ashfame mentioned this pull request Mar 28, 2025

PG Onboarding flow: playground GET param is lost for logged out users #101948

Closed

	const [ , setSearchParams ] = useSearchParams();
	const [ setSearchParams ] = useSearchParams();

		url.searchParams.set( 'playground', playgroundId );
		window.history.replaceState( {}, '', url.toString() );

Conversation

ashfame commented Mar 28, 2025

Proposed Changes

Testing Instructions

Pre-merge Checklist

Uh oh!

github-actions bot commented Mar 28, 2025 • edited by matticbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

matticbot commented Mar 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matticbot commented Mar 28, 2025

Uh oh!

zaerl left a comment

Choose a reason for hiding this comment

Uh oh!

ashfame commented Mar 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

github-actions bot commented Mar 28, 2025 •

edited by matticbot

Loading