Update wizard UI and improve e2e tests

assets/src/setup/__mocks__/amp-setup.js

@@ -1,5 +1,8 @@
 module.exports = {
 	APP_ROOT_ID: 'amp-setup',
+	CURRENT_THEME: {
+		name: 'Twenty Twenty',
+	},
 	EXIT_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',

assets/src/setup/components/nav/index.js

@@ -33,18 +33,20 @@ export function Nav( { exitLink } ) {
 		<div className="amp-setup-nav">
 			<div className="amp-setup-nav__inner">
 				<div className="amp-setup-nav__close">
-					<Button isLink href={ exitLink }>
-						<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" />
-							</mask>
-							<g mask="url(#close-icon)">
-								<rect width="24" height="24" transform="matrix(-1 0 0 1 24.895 0.71875)" fill="#2459E7" />
-							</g>
-						</svg>
+					{ ! isLastPage && (
+						<Button isLink href={ exitLink }>
+							<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" />
+								</mask>
+								<g mask="url(#close-icon)">
+									<rect width="24" height="24" transform="matrix(-1 0 0 1 24.895 0.71875)" fill="#2459E7" />
+								</g>
+							</svg>
 
-						{ __( 'Close', 'amp' ) }
-					</Button>
+							{ __( 'Close', 'amp' ) }
+						</Button>
+					) }
 				</div>
 				<div className="amp-setup-nav__prev-next">
 					{ 1 > activePageIndex

assets/src/setup/components/nav/test/index.js

@@ -1,9 +1,9 @@
-
 /**
  * External dependencies
  */
 import { act } from 'react-dom/test-utils';
 import { create } from 'react-test-renderer';
+import PropTypes from 'prop-types';
 
 /**
  * WordPress dependencies
@@ -29,7 +29,24 @@ const getNavButtons = ( containerElement ) => ( {
 } );
 
 const MyPageComponent = () => <div />;
-const testPages = [ { PageComponent: MyPageComponent, slug: 'slug', title: 'Page 0' }, { PageComponent: MyPageComponent, slug: 'slug-2', title: 'Page 1' } ];
+const testPages = [
+	{ PageComponent: MyPageComponent, slug: 'slug', title: 'Page 0' },
+	{ PageComponent: MyPageComponent, slug: 'slug-2', title: 'Page 1' },
+];
+
+const Providers = ( { children, pages } ) => (
+	<OptionsContextProvider>
+		<UserContextProvider>
+			<NavigationContextProvider pages={ pages }>
+				{ children }
+			</NavigationContextProvider>
+		</UserContextProvider>
+	</OptionsContextProvider>
+);
+Providers.propTypes = {
+	children: PropTypes.any,
+	pages: PropTypes.array,
+};
 
 describe( 'Nav', () => {
 	beforeEach( () => {
@@ -44,27 +61,19 @@ describe( 'Nav', () => {
 
 	it( 'matches snapshot', () => {
 		const wrapper = create(
-			<OptionsContextProvider>
-				<UserContextProvider>
-					<NavigationContextProvider pages={ testPages }>
-						<Nav exitLink="http://site.test" />
-					</NavigationContextProvider>
-				</UserContextProvider>
-			</OptionsContextProvider>,
+			<Providers pages={ testPages }>
+				<Nav exitLink="http://site.test" />
+			</Providers>,
 		);
 		expect( wrapper.toJSON() ).toMatchSnapshot();
 	} );
 
 	it( 'hides previous button on first page', () => {
 		act( () => {
 			render(
-				<OptionsContextProvider>
-					<UserContextProvider>
-						<NavigationContextProvider pages={ testPages }>
-							<Nav exitLink="http://site.test" />
-						</NavigationContextProvider>
-					</UserContextProvider>
-				</OptionsContextProvider>,
+				<Providers pages={ testPages }>
+					<Nav exitLink="http://site.test" />
+				</Providers>,
 				container,
 			);
 		} );
@@ -74,4 +83,48 @@ describe( 'Nav', () => {
 		expect( prevButton ).toBeNull();
 		expect( nextButton ).not.toBeNull();
 	} );
+
+	it( 'changes next button to "Finish" on last page', () => {
+		act( () => {
+			render(
+				<Providers pages={ testPages }>
+					<Nav exitLink="http://site.test" />
+				</Providers>,
+				container,
+			);
+		} );
+
+		const { nextButton } = getNavButtons( container );
+
+		expect( nextButton.textContent ).toBe( 'Next' );
+
+		act( () => {
+			nextButton.dispatchEvent( new global.MouseEvent( 'click', { bubbles: true } ) );
+		} );
+
+		expect( nextButton.textContent ).toBe( 'Finish' );
+	} );
+
+	it( 'close button hides on last page', () => {
+		act( () => {
+			render(
+				<Providers pages={ testPages }>
+					<Nav exitLink="http://site.test" />
+				</Providers>,
+				container,
+			);
+		} );
+
+		const { nextButton } = getNavButtons( container );
+		let closeButton = container.querySelector( '.amp-setup-nav__close a' );
+
+		expect( closeButton ).not.toBeNull();
+
+		act( () => {
+			nextButton.dispatchEvent( new global.MouseEvent( 'click', { bubbles: true } ) );
+		} );
+
+		closeButton = container.querySelector( '.amp-setup-nav__close a' );
+		expect( closeButton ).toBeNull();
+	} );
 } );

assets/src/setup/components/options-context-provider.js

@@ -37,7 +37,7 @@ export function OptionsContextProvider( { children, optionsRestEndpoint } ) {
 	const [ fetchingOptions, setFetchingOptions ] = useState( false );
 	const [ savingOptions, setSavingOptions ] = useState( false );
 	const [ didSaveOptions, setDidSaveOptions ] = useState( false );
-	const [ originalOptions, setOriginalOptions ] = useState( null );
+	const [ originalOptions, setOriginalOptions ] = useState( {} );
 
 	const { setError } = useError();
 
@@ -47,6 +47,42 @@ export function OptionsContextProvider( { children, optionsRestEndpoint } ) {
 		hasUnmounted.current = true;
 	}, [] );
 
+	/**
+	 * Fetches options.
+	 */
+	useEffect( () => {
+		if ( Object.keys( originalOptions ).length || fetchingOptions ) {
+			return;
+		}
+
+		/**
+		 * Fetches plugin options from the REST endpoint.
+		 */
+		( async () => {
+			setFetchingOptions( true );
+
+			try {
+				const fetchedOptions = await apiFetch( { url: optionsRestEndpoint } );
+
+				if ( true === hasUnmounted.current ) {
+					return;
+				}
+
+				if ( fetchedOptions.wizard_completed === false ) {
+					fetchedOptions.mobile_redirect = true;
+					fetchedOptions.reader_theme = null;
+				}
+
+				setOriginalOptions( fetchedOptions );
+			} catch ( e ) {
+				setError( e );
+				return;
+			}
+
+			setFetchingOptions( false );
+		} )();
+	}, [ fetchingOptions, originalOptions, optionsRestEndpoint, setError ] );
+
 	/**
 	 * Sends options to the REST endpoint to be saved.
 	 *
@@ -103,45 +139,6 @@ export function OptionsContextProvider( { children, optionsRestEndpoint } ) {
 		setDidSaveOptions( false );
 	};
 
-	useEffect( () => {
-		if ( Object.keys( updates ).length || fetchingOptions ) {
-			return;
-		}
-
-		/**
-		 * Fetches plugin options from the REST endpoint.
-		 */
-		( async () => {
-			setFetchingOptions( true );
-
-			try {
-				const fetchedOptions = await apiFetch( { url: optionsRestEndpoint } );
-
-				if ( true === hasUnmounted.current ) {
-					return;
-				}
-
-				setOriginalOptions( fetchedOptions );
-
-				const initialUpdates = {
-					wizard_completed: true,
-				};
-
-				if ( fetchedOptions.wizard_completed === false ) {
-					initialUpdates.mobile_redirect = true;
-					initialUpdates.reader_theme = null;
-				}
-
-				setUpdates( initialUpdates );
-			} catch ( e ) {
-				setError( e );
-				return;
-			}
-
-			setFetchingOptions( false );
-		} )();
-	}, [ fetchingOptions, updates, optionsRestEndpoint, setError ] );
-
 	// Allows an item in the updates object to be removed.
 	const unsetOption = useCallback( ( option ) => {
 		const newOptions = { ...updates };

assets/src/setup/components/reader-themes-context-provider.js

@@ -15,43 +15,64 @@ import PropTypes from 'prop-types';
  */
 import { useError } from '../utils/use-error';
 import { Options } from './options-context-provider';
+import { Navigation } from './navigation-context-provider';
 
 export const ReaderThemes = createContext();
 
 /**
  * Context provider for options retrieval and updating.
  *
  * @param {Object} props Component props.
+ * @param {string} props.currentTheme The theme currently active on the site.
  * @param {string} props.wpAjaxUrl WP AJAX URL.
  * @param {?any} props.children Component children.
  * @param {string} props.readerThemesEndpoint REST endpoint to fetch reader themes.
  * @param {string} props.updatesNonce Nonce for the AJAX request to install a theme.
  */
-export function ReaderThemesContextProvider( { wpAjaxUrl, children, readerThemesEndpoint, updatesNonce } ) {
+export function ReaderThemesContextProvider( { wpAjaxUrl, children, currentTheme, readerThemesEndpoint, updatesNonce } ) {
+	const { setError } = useError();
+
 	const [ themes, setThemes ] = useState( null );
 	const [ fetchingThemes, setFetchingThemes ] = useState( false );
 	const [ downloadingTheme, setDownloadingTheme ] = useState( false );
 	const [ downloadedTheme, setDownloadedTheme ] = useState( false );
 
-	/**
-	 * Handle downloaded theme errors separately from normal error handling because we don't want it to break the application
-	 * after settings have already been saved.
-	 */
-	const [ downloadingThemeError, setDownloadingThemeError ] = useState( null );
-
-	const { setError } = useError();
-
-	const { editedOptions, savingOptions } = useContext( Options );
+	const { editedOptions, updateOptions, savingOptions } = useContext( Options );
 	const { reader_theme: readerTheme, theme_support: themeSupport } = editedOptions;
 
 	// This component sets state inside async functions. Use this ref to prevent state updates after unmount.
 	const hasUnmounted = useRef( false );
+	useEffect( () => () => {
+		hasUnmounted.current = true;
+	}, [] );
 
+	/**
+	 * The active reader theme.
+	 */
 	const selectedTheme = useMemo(
-		() => themes ? themes.find( ( { slug } ) => slug === readerTheme ) : null,
+		() => themes ? themes.find( ( { slug } ) => slug === readerTheme ) || { name: null } : { name: null },
 		[ readerTheme, themes ],
 	);
 
+	/**
+	 * When reader mode was selected selected and the user chooses the currently active theme as the reader theme,
+	 * we will override their choice with transitional.
+	 */
+	const [ readerModeWasOverridden, setReaderModeWasOverridden ] = useState( false );
+	const { currentPage: { slug: currentPageSlug } } = useContext( Navigation );
+	useEffect( () => {
+		if ( 'summary' === currentPageSlug && 'reader' === themeSupport && selectedTheme.name === currentTheme.name ) {
+			updateOptions( { theme_support: 'transitional' } );
+			setReaderModeWasOverridden( true );
+		}
+	}, [ selectedTheme.name, currentTheme.name, themeSupport, currentPageSlug, updateOptions ] );
+
+	/**
+	 * Handle downloaded theme errors separately from normal error handling because we don't want it to break the application
+	 * after settings have already been saved.
+	 */
+	const [ downloadingThemeError, setDownloadingThemeError ] = useState( null );
+
 	/**
 	 * Downloads the selected reader theme, if necessary, when options are saved.
 	 */
@@ -116,7 +137,7 @@ export function ReaderThemesContextProvider( { wpAjaxUrl, children, readerThemes
 	 * Fetches theme data when needed.
 	 */
 	useEffect( () => {
-		if ( fetchingThemes || ! readerThemesEndpoint || themes || 'reader' !== themeSupport ) {
+		if ( fetchingThemes || ! readerThemesEndpoint || themes || 'standard' === themeSupport ) {
 			return;
 		}
 
@@ -144,18 +165,16 @@ export function ReaderThemesContextProvider( { wpAjaxUrl, children, readerThemes
 		} )();
 	}, [ fetchingThemes, readerThemesEndpoint, setError, themes, themeSupport ] );
 
-	useEffect( () => () => {
-		hasUnmounted.current = true;
-	}, [] );
-
 	return (
 		<ReaderThemes.Provider
 			value={
 				{
+					currentTheme,
 					downloadedTheme,
 					downloadingTheme,
 					downloadingThemeError,
 					fetchingThemes,
+					readerModeWasOverridden,
 					themes,
 				}
 			}
@@ -166,8 +185,11 @@ export function ReaderThemesContextProvider( { wpAjaxUrl, children, readerThemes
 }
 
 ReaderThemesContextProvider.propTypes = {
-	wpAjaxUrl: PropTypes.string.isRequired,
 	children: PropTypes.any,
+	currentTheme: PropTypes.shape( {
+		name: PropTypes.string.isRequired,
+	} ).isRequired,
 	readerThemesEndpoint: PropTypes.string.isRequired,
 	updatesNonce: PropTypes.string.isRequired,
+	wpAjaxUrl: PropTypes.string.isRequired,
 };