Skip to content

Commit

Permalink
Onboarding wizard close link: return to previous location
Browse files Browse the repository at this point in the history
  • Loading branch information
johnwatkins0 committed Jul 6, 2020
1 parent 11d6d6b commit 436a87d
Show file tree
Hide file tree
Showing 10 changed files with 83 additions and 26 deletions.
3 changes: 2 additions & 1 deletion assets/src/setup/__mocks__/amp-setup.js
Original file line number Diff line number Diff line change
@@ -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: '',
Expand Down
8 changes: 4 additions & 4 deletions assets/src/setup/components/error-screen/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand All @@ -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>
Expand All @@ -47,5 +47,5 @@ ErrorScreen.propTypes = {
error: PropTypes.shape( {
message: PropTypes.string,
} ).isRequired,
exitLink: PropTypes.string.isRequired,
finishLink: PropTypes.string.isRequired,
};
2 changes: 1 addition & 1 deletion assets/src/setup/components/error-screen/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
} );
Expand Down
12 changes: 7 additions & 5 deletions assets/src/setup/components/nav/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand All @@ -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" />
Expand Down Expand Up @@ -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 }
>
Expand All @@ -92,5 +93,6 @@ export function Nav( { exitLink } ) {
}

Nav.propTypes = {
exitLink: PropTypes.string.isRequired,
closeLink: PropTypes.string.isRequired,
finishLink: PropTypes.string.isRequired,
};
8 changes: 4 additions & 4 deletions assets/src/setup/components/nav/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -72,7 +72,7 @@ describe( 'Nav', () => {
act( () => {
render(
<Providers pages={ testPages }>
<Nav exitLink="http://site.test" />
<Nav finishLink="http://site.test" />
</Providers>,
container,
);
Expand All @@ -88,7 +88,7 @@ describe( 'Nav', () => {
act( () => {
render(
<Providers pages={ testPages }>
<Nav exitLink="http://site.test" />
<Nav finishLink="http://site.test" />
</Providers>,
container,
);
Expand All @@ -109,7 +109,7 @@ describe( 'Nav', () => {
act( () => {
render(
<Providers pages={ testPages }>
<Nav exitLink="http://site.test" />
<Nav finishLink="http://site.test" />
</Providers>,
container,
);
Expand Down
7 changes: 4 additions & 3 deletions assets/src/setup/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -98,7 +99,7 @@ class ErrorBoundary extends Component {

if ( error ) {
return (
<ErrorScreen error={ error } exitLink={ EXIT_LINK } />
<ErrorScreen error={ error } finishLink={ FINISH_LINK } />
);
}

Expand All @@ -113,7 +114,7 @@ domReady( () => {
render(
<ErrorBoundary>
<Providers>
<SetupWizard exitLink={ EXIT_LINK } />
<SetupWizard closeLink={ CLOSE_LINK } finishLink={ FINISH_LINK } />
</Providers>
</ErrorBoundary>,
root,
Expand Down
11 changes: 7 additions & 4 deletions assets/src/setup/setup-wizard.js
Original file line number Diff line number Diff line change
Expand Up @@ -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( () => () => (
Expand Down Expand Up @@ -75,7 +76,8 @@ export function SetupWizard( { exitLink } ) {
</Panel>
<Nav
activePageIndex={ activePageIndex }
exitLink={ exitLink }
closeLink={ closeLink }
finishLink={ finishLink }
moveBack={ moveBack }
moveForward={ moveForward }
pages={ pages }
Expand All @@ -88,5 +90,6 @@ export function SetupWizard( { exitLink } ) {
}

SetupWizard.propTypes = {
exitLink: PropTypes.string.isRequired,
closeLink: PropTypes.string.isRequired,
finishLink: PropTypes.string.isRequired,
};
Original file line number Diff line number Diff line change
Expand Up @@ -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' ),
Expand All @@ -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' ),
Expand Down
40 changes: 40 additions & 0 deletions tests/e2e/specs/amp-onboarding/exit-links.js
Original file line number Diff line number Diff line change
@@ -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();
} );
} );
11 changes: 9 additions & 2 deletions tests/e2e/utils/onboarding-wizard-utils.js
Original file line number Diff line number Diff line change
@@ -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' } );
Expand All @@ -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' );
}
Expand Down

0 comments on commit 436a87d

Please sign in to comment.