From 436a87df4f1a8c1e49a8aea434bef5fe4177566e Mon Sep 17 00:00:00 2001
From: John Watkins <johnwatkins0@gmail.com>
Date: Mon, 6 Jul 2020 10:03:53 -0500
Subject: [PATCH] Onboarding wizard close link: return to previous location

---
 assets/src/setup/__mocks__/amp-setup.js       |  3 +-
 .../setup/components/error-screen/index.js    |  8 ++--
 .../components/error-screen/test/index.js     |  2 +-
 assets/src/setup/components/nav/index.js      | 12 +++---
 assets/src/setup/components/nav/test/index.js |  8 ++--
 assets/src/setup/index.js                     |  7 ++--
 assets/src/setup/setup-wizard.js              | 11 +++--
 .../class-amp-setup-wizard-submenu-page.php   |  7 +++-
 tests/e2e/specs/amp-onboarding/exit-links.js  | 40 +++++++++++++++++++
 tests/e2e/utils/onboarding-wizard-utils.js    | 11 ++++-
 10 files changed, 83 insertions(+), 26 deletions(-)
 create mode 100644 tests/e2e/specs/amp-onboarding/exit-links.js

diff --git a/assets/src/setup/__mocks__/amp-setup.js b/assets/src/setup/__mocks__/amp-setup.js
index d44eb979975..2c45a91f103 100644
--- a/assets/src/setup/__mocks__/amp-setup.js
+++ b/assets/src/setup/__mocks__/amp-setup.js
@@ -1,9 +1,10 @@
 module.exports = {
 	APP_ROOT_ID: 'amp-setup',
+	CLOSE_LINK: 'http://site.test/wp-admin/?page=amp-options',
 	CURRENT_THEME: {
 		name: 'Twenty Twenty',
 	},
-	EXIT_LINK: 'http://site.test/wp-admin/?page=amp-options',
+	FINISH_LINK: 'http://site.test/wp-admin/?page=amp-options',
 	OPTIONS_REST_ENDPOINT: 'http://site.test/wp-json/amp/v1/options',
 	READER_THEMES_REST_ENDPOINT: 'http://site.test/wp-json/amp/v1/reader-themes',
 	UPDATES_NONCE: '',
diff --git a/assets/src/setup/components/error-screen/index.js b/assets/src/setup/components/error-screen/index.js
index 1f715849468..ffceb093d13 100644
--- a/assets/src/setup/components/error-screen/index.js
+++ b/assets/src/setup/components/error-screen/index.js
@@ -19,9 +19,9 @@ import './style.css';
  *
  * @param {Object} props Component props.
  * @param {Object} props.error Error object containing a message string.
- * @param {string} props.exitLink The link to return to the admin.
+ * @param {string} props.finishLink The link to return to the admin.
  */
-export function ErrorScreen( { error, exitLink } ) {
+export function ErrorScreen( { error, finishLink } ) {
 	return (
 		<div className="error-screen-container">
 			<Panel className="error-screen">
@@ -34,7 +34,7 @@ export function ErrorScreen( { error, exitLink } ) {
 						dangerouslySetInnerHTML={ { __html: error.message || __( 'There was an error loading the setup wizard.', 'amp' ) } }
 					/>
 					{ ' ' }
-					<a href={ exitLink }>
+					<a href={ finishLink }>
 						{ __( 'Return to AMP settings.', 'amp' ) }
 					</a>
 				</p>
@@ -47,5 +47,5 @@ ErrorScreen.propTypes = {
 	error: PropTypes.shape( {
 		message: PropTypes.string,
 	} ).isRequired,
-	exitLink: PropTypes.string.isRequired,
+	finishLink: PropTypes.string.isRequired,
 };
diff --git a/assets/src/setup/components/error-screen/test/index.js b/assets/src/setup/components/error-screen/test/index.js
index 96e4fafa23c..53ba9aa7299 100644
--- a/assets/src/setup/components/error-screen/test/index.js
+++ b/assets/src/setup/components/error-screen/test/index.js
@@ -12,7 +12,7 @@ import { ErrorScreen } from '..';
 describe( 'ErrorScreen', () => {
 	it( 'matches snapshot', () => {
 		const wrapper = create(
-			<ErrorScreen exitLink={ 'http://my-exit-link.com' } error={ { message: 'The application failed' } } />,
+			<ErrorScreen finishLink={ 'http://my-exit-link.com' } error={ { message: 'The application failed' } } />,
 		);
 		expect( wrapper.toJSON() ).toMatchSnapshot();
 	} );
diff --git a/assets/src/setup/components/nav/index.js b/assets/src/setup/components/nav/index.js
index 929c7c96949..f951dedfb8f 100644
--- a/assets/src/setup/components/nav/index.js
+++ b/assets/src/setup/components/nav/index.js
@@ -22,9 +22,10 @@ import { User } from '../user-context-provider';
  * Navigation component.
  *
  * @param {Object} props Component props.
- * @param {string} props.exitLink Link to exit the application.
+ * @param {string} props.closeLink Link to return to previous user location.
+ * @param {string} props.finishLink Link to exit the application.
  */
-export function Nav( { exitLink } ) {
+export function Nav( { closeLink, finishLink } ) {
 	const { activePageIndex, canGoForward, isLastPage, moveBack, moveForward } = useContext( Navigation );
 	const { savingOptions } = useContext( Options );
 	const { savingDeveloperToolsOption } = useContext( User );
@@ -34,7 +35,7 @@ export function Nav( { exitLink } ) {
 			<div className="amp-setup-nav__inner">
 				<div className="amp-setup-nav__close">
 					{ ! isLastPage && (
-						<Button isLink href={ exitLink }>
+						<Button isLink href={ closeLink }>
 							<svg width="25" height="25" viewBox="0 0 25 25" fill="none" xmlns="http://www.w3.org/2000/svg">
 								<mask id="close-icon" mask-type="alpha" maskUnits="userSpaceOnUse" x="3" y="3" width="19" height="19">
 									<path fillRule="evenodd" clipRule="evenodd" d="M19.895 3.71875H5.89502C4.79502 3.71875 3.89502 4.61875 3.89502 5.71875V19.7188C3.89502 20.8188 4.79502 21.7188 5.89502 21.7188H19.895C21.005 21.7188 21.895 20.8188 21.895 19.7188V15.7188H19.895V19.7188H5.89502V5.71875H19.895V9.71875H21.895V5.71875C21.895 4.61875 21.005 3.71875 19.895 3.71875ZM13.395 17.7188L14.805 16.3088L12.225 13.7188H21.895V11.7188H12.225L14.805 9.12875L13.395 7.71875L8.39502 12.7188L13.395 17.7188Z" fill="white" />
@@ -71,7 +72,7 @@ export function Nav( { exitLink } ) {
 
 					<Button
 						disabled={ ! canGoForward || savingOptions || savingDeveloperToolsOption }
-						href={ isLastPage && ! savingDeveloperToolsOption && ! savingOptions ? exitLink : undefined }
+						href={ isLastPage && ! savingDeveloperToolsOption && ! savingOptions ? finishLink : undefined }
 						isPrimary
 						onClick={ moveForward }
 					>
@@ -92,5 +93,6 @@ export function Nav( { exitLink } ) {
 }
 
 Nav.propTypes = {
-	exitLink: PropTypes.string.isRequired,
+	closeLink: PropTypes.string.isRequired,
+	finishLink: PropTypes.string.isRequired,
 };
diff --git a/assets/src/setup/components/nav/test/index.js b/assets/src/setup/components/nav/test/index.js
index 0d63e6b810e..cf7d43ae203 100644
--- a/assets/src/setup/components/nav/test/index.js
+++ b/assets/src/setup/components/nav/test/index.js
@@ -62,7 +62,7 @@ describe( 'Nav', () => {
 	it( 'matches snapshot', () => {
 		const wrapper = create(
 			<Providers pages={ testPages }>
-				<Nav exitLink="http://site.test" />
+				<Nav finishLink="http://site.test" />
 			</Providers>,
 		);
 		expect( wrapper.toJSON() ).toMatchSnapshot();
@@ -72,7 +72,7 @@ describe( 'Nav', () => {
 		act( () => {
 			render(
 				<Providers pages={ testPages }>
-					<Nav exitLink="http://site.test" />
+					<Nav finishLink="http://site.test" />
 				</Providers>,
 				container,
 			);
@@ -88,7 +88,7 @@ describe( 'Nav', () => {
 		act( () => {
 			render(
 				<Providers pages={ testPages }>
-					<Nav exitLink="http://site.test" />
+					<Nav finishLink="http://site.test" />
 				</Providers>,
 				container,
 			);
@@ -109,7 +109,7 @@ describe( 'Nav', () => {
 		act( () => {
 			render(
 				<Providers pages={ testPages }>
-					<Nav exitLink="http://site.test" />
+					<Nav finishLink="http://site.test" />
 				</Providers>,
 				container,
 			);
diff --git a/assets/src/setup/index.js b/assets/src/setup/index.js
index 014273f7be7..588fce1c8ab 100644
--- a/assets/src/setup/index.js
+++ b/assets/src/setup/index.js
@@ -10,8 +10,9 @@ import '@wordpress/components/build-style/style.css';
  */
 import {
 	APP_ROOT_ID,
+	CLOSE_LINK,
 	CURRENT_THEME,
-	EXIT_LINK,
+	FINISH_LINK,
 	OPTIONS_REST_ENDPOINT,
 	READER_THEMES_REST_ENDPOINT,
 	UPDATES_NONCE,
@@ -98,7 +99,7 @@ class ErrorBoundary extends Component {
 
 		if ( error ) {
 			return (
-				<ErrorScreen error={ error } exitLink={ EXIT_LINK } />
+				<ErrorScreen error={ error } finishLink={ FINISH_LINK } />
 			);
 		}
 
@@ -113,7 +114,7 @@ domReady( () => {
 		render(
 			<ErrorBoundary>
 				<Providers>
-					<SetupWizard exitLink={ EXIT_LINK } />
+					<SetupWizard closeLink={ CLOSE_LINK } finishLink={ FINISH_LINK } />
 				</Providers>
 			</ErrorBoundary>,
 			root,
diff --git a/assets/src/setup/setup-wizard.js b/assets/src/setup/setup-wizard.js
index 7c7a802134d..e388d7d72b0 100644
--- a/assets/src/setup/setup-wizard.js
+++ b/assets/src/setup/setup-wizard.js
@@ -38,9 +38,10 @@ function PageComponentSideEffects( { children } ) {
  * Setup wizard root component.
  *
  * @param {Object} props Component props.
- * @param {Array} props.exitLink Exit link.
+ * @param {string} props.closeLink Link to return to previous user location.
+ * @param {string} props.finishLink Exit link.
  */
-export function SetupWizard( { exitLink } ) {
+export function SetupWizard( { closeLink, finishLink } ) {
 	const { activePageIndex, currentPage: { title, PageComponent, showTitle }, moveBack, moveForward, pages } = useContext( Navigation );
 
 	const PageComponentWithSideEffects = useMemo( () => () => (
@@ -75,7 +76,8 @@ export function SetupWizard( { exitLink } ) {
 					</Panel>
 					<Nav
 						activePageIndex={ activePageIndex }
-						exitLink={ exitLink }
+						closeLink={ closeLink }
+						finishLink={ finishLink }
 						moveBack={ moveBack }
 						moveForward={ moveForward }
 						pages={ pages }
@@ -88,5 +90,6 @@ export function SetupWizard( { exitLink } ) {
 }
 
 SetupWizard.propTypes = {
-	exitLink: PropTypes.string.isRequired,
+	closeLink: PropTypes.string.isRequired,
+	finishLink: PropTypes.string.isRequired,
 };
diff --git a/includes/options/views/class-amp-setup-wizard-submenu-page.php b/includes/options/views/class-amp-setup-wizard-submenu-page.php
index 922836369e4..2cf50532e88 100644
--- a/includes/options/views/class-amp-setup-wizard-submenu-page.php
+++ b/includes/options/views/class-amp-setup-wizard-submenu-page.php
@@ -140,16 +140,18 @@ public function enqueue_assets( $hook_suffix ) {
 		$theme           = wp_get_theme();
 		$is_reader_theme = in_array( get_stylesheet(), wp_list_pluck( ( new AMP_Reader_Themes() )->get_themes(), 'slug' ), true );
 
+		$exit_link = menu_page_url( AMP_Options_Manager::OPTION_NAME, false );
+
 		$setup_wizard_data = [
 			'AMP_OPTIONS_KEY'                    => AMP_Options_Manager::OPTION_NAME,
 			'APP_ROOT_ID'                        => self::APP_ROOT_ID,
 			'CUSTOMIZER_LINK'                    => add_query_arg(
 				[
-					'return' => rawurlencode( menu_page_url( AMP_Options_Manager::OPTION_NAME, false ) ),
+					'return' => rawurlencode( $exit_link ),
 				],
 				admin_url( 'customize.php' )
 			),
-			'EXIT_LINK'                          => menu_page_url( AMP_Options_Manager::OPTION_NAME, false ),
+			'CLOSE_LINK'                         => wp_get_referer() ?: $exit_link,
 			// @todo As of June 2020, an upcoming WP release will allow this to be retrieved via REST.
 			'CURRENT_THEME'                      => [
 				'name'            => $theme->get( 'Name' ),
@@ -158,6 +160,7 @@ public function enqueue_assets( $hook_suffix ) {
 				'screenshot'      => $theme->get_screenshot(),
 				'url'             => $theme->get( 'ThemeURI' ),
 			],
+			'FINISH_LINK'                        => $exit_link,
 			'OPTIONS_REST_ENDPOINT'              => rest_url( 'amp/v1/options' ),
 			'READER_THEMES_REST_ENDPOINT'        => rest_url( 'amp/v1/reader-themes' ),
 			'UPDATES_NONCE'                      => wp_create_nonce( 'updates' ),
diff --git a/tests/e2e/specs/amp-onboarding/exit-links.js b/tests/e2e/specs/amp-onboarding/exit-links.js
new file mode 100644
index 00000000000..3b480076256
--- /dev/null
+++ b/tests/e2e/specs/amp-onboarding/exit-links.js
@@ -0,0 +1,40 @@
+/**
+ * WordPress dependencies
+ */
+const { visitAdminPage } = require( '@wordpress/e2e-test-utils/build/visit-admin-page' );
+/**
+ * Internal dependencies
+ */
+const { goToOnboardingWizard, completeWizard, cleanUpWizard } = require( '../../utils/onboarding-wizard-utils' );
+
+describe( 'Onboarding wizard exit links', () => {
+	it( 'if no previous page, returns to settings when clicking close', async () => {
+		await goToOnboardingWizard();
+		await expect( page ).toClick( 'a', { text: 'Close' } );
+		await page.waitForSelector( '.wp-admin' );
+		await expect( page ).toMatchElement( 'h1', { text: 'AMP Settings' } );
+	} );
+
+	it( 'returns to previous page when clicking close', async () => {
+		await visitAdminPage( 'index.php' );
+		await page.waitForSelector( '.wp-admin' );
+
+		await page.evaluate( () => {
+			document.querySelector( 'a[href="admin.php?page=amp-setup"]' ).click();
+		} );
+		await page.waitForSelector( '#amp-setup' );
+		await expect( page ).toClick( 'a', { text: 'Close' } );
+		await page.waitForSelector( '.wp-admin' );
+		await expect( page ).toMatchElement( 'h1', { text: 'Dashboard' } );
+	} );
+
+	it( 'goes to settings when clicking finish', async () => {
+		await completeWizard( { mode: 'standard' } );
+		await expect( page ).toClick( 'a', { text: 'Finish' } );
+		await page.waitForSelector( '.wp-admin' );
+		await expect( page ).toMatchElement( 'h1', { text: 'AMP Settings' } );
+
+		await goToOnboardingWizard();
+		await cleanUpWizard();
+	} );
+} );
diff --git a/tests/e2e/utils/onboarding-wizard-utils.js b/tests/e2e/utils/onboarding-wizard-utils.js
index 09954173ce1..083983392d1 100644
--- a/tests/e2e/utils/onboarding-wizard-utils.js
+++ b/tests/e2e/utils/onboarding-wizard-utils.js
@@ -1,11 +1,18 @@
 /**
  * WordPress dependencies
  */
-import { visitAdminPage } from '@wordpress/e2e-test-utils/build/visit-admin-page';
+import { visitAdminPage, isCurrentURL } from '@wordpress/e2e-test-utils';
 
 export const NEXT_BUTTON_SELECTOR = '.amp-setup-nav__prev-next button.is-primary';
 export const PREV_BUTTON_SELECTOR = '.amp-setup-nav__prev-next button:not(.is-primary)';
 
+export async function goToOnboardingWizard() {
+	if ( ! isCurrentURL( 'admin.php', 'page=amp-setup' ) ) {
+		await visitAdminPage( 'admin.php', 'page=amp-setup' );
+	}
+	await page.waitForSelector( '#amp-setup' );
+}
+
 export async function clickNextButton() {
 	await page.waitForSelector( `${ NEXT_BUTTON_SELECTOR }:not([disabled])` );
 	await expect( page ).toClick( 'button', { text: 'Next' } );
@@ -17,7 +24,7 @@ export async function clickPrevButton() {
 }
 
 export async function moveToTechnicalScreen() {
-	await visitAdminPage( 'admin.php', 'page=amp-setup' );
+	await goToOnboardingWizard();
 	await clickNextButton();
 	await page.waitForSelector( '.technical-background-option' );
 }