If we can simply add a "by installing Jetpack and WooCommerce Services you agree to the TOS" line somewhere, and preserve plugin installation in this step, that would be worth it imo.

Closing and reopening to kick off a Travis CI build. Sorry for the noise!

This needs to be rebased from main to fix the failing test.

Thanks for the review, @psealock! I wasn't able to reproduce the address bug. Any other tricks that might help? What country did you have selected?

What was happening to cause the redirect?

Your guess (discussed in Slack) as to why this was happening was correct. The previous method of checking was to monitor changes to options and redirect when a change occurred.

componentDidUpdate() {
	...

	const {	
		woocommerce_calc_taxes: prevCalcTaxes,	
	} = prevProps.generalSettings;	
	if ( prevCalcTaxes === 'no' && calcTaxes === 'yes' ) {	
		window.location = getAdminLink(	
			'admin.php?page=wc-settings&tab=tax&section=standard'	
		);	
	}
}

Now that we have wp data actions returning promises, the await began to work when updating options and the component would update tax settings and redirect to the core tax screen before unmounting.

The logic in this task was generally a mess from previous development without promises, so I took this as an opportunity to refactor the whole thing.

We have quite a few other places in onboarding that are still in need of this, which I'll make note of in #2959. Besides async patterns that could use some patching from the days of wc-api, we also have some leftover error handling that won't work as expected.

Also noting I added the change that @jameskoster requested in 024cf76. This shows the plugin step if plugins are needed or if opt-in to TOS is needed to enable automatic taxes.

Testing steps should be mostly identical, but disable Jetpack or WCS to also note that the plugins step will appear.

I wasn't able to reproduce the address bug.

I tried again and wasn't able to do so consistently. I'm not sure what changed, but I tried it maybe 10 times and reproduced once. I'm ok with shipping this and seeing if we get reports of it happening in the wild.

the component would update tax settings and redirect to the core tax screen before unmounting.

Thats what I figured, but was thrown off by &section=standard because I didn't see that query param in my testing when redirected. 🤷 Thanks for the explanation and fix.

disable Jetpack or WCS to also note that the plugins step will appear.

This part is working well for me. I did notice one thing. When click "Install and enable" or "Continue" when having to enter my address, the Success screen showed and automatically moved on. Maybe thats because I already chose that option?

This part is working well for me. I did notice one thing. When click "Install and enable" or "Continue" when having to enter my address, the Success screen showed and automatically moved on. Maybe thats because I already chose that option?

Great catch! I think this is happening because we call completeStep() and there's a generic catch to send us back to the task list if there are no more steps. Since the success screen isn't really part of the steps, it assumes the store address is the last step and redirects back to the task list.

I removed that snippet in ab1f19a as every step has a way of manually exiting this flow. Testing this seems to work well, but could use your 👀 to double check.

Nice! Would it be possible to use the caption Text style for this? And set the color to $dark-gray-300?

Thanks for taking the time to look at this @jameskoster! Updated styling for captions in 3567fbf