Override reader mode selection when reader theme matches active theme

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

@@ -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,39 +139,6 @@ export function OptionsContextProvider( { children, optionsRestEndpoint } ) {
 		setDidSaveOptions( false );
 	};
 
-	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 ] );
-
 	// 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,
 };

assets/src/setup/index.js

@@ -8,7 +8,7 @@ import '@wordpress/components/build-style/style.css';
 /**
  * External dependencies
  */
-import { APP_ROOT_ID, EXIT_LINK, OPTIONS_REST_ENDPOINT, READER_THEMES_REST_ENDPOINT, UPDATES_NONCE, USER_FIELD_DEVELOPER_TOOLS_ENABLED, USER_REST_ENDPOINT } from 'amp-setup'; // From WP inline script.
+import { APP_ROOT_ID, CURRENT_THEME, EXIT_LINK, OPTIONS_REST_ENDPOINT, READER_THEMES_REST_ENDPOINT, UPDATES_NONCE, USER_FIELD_DEVELOPER_TOOLS_ENABLED, USER_REST_ENDPOINT } from 'amp-setup'; // From WP inline script.
 import PropTypes from 'prop-types';
 
 /**
@@ -43,6 +43,7 @@ export function Providers( { children } ) {
 			>
 				<NavigationContextProvider pages={ PAGES }>
 					<ReaderThemesContextProvider
+						currentTheme={ CURRENT_THEME }
 						wpAjaxUrl={ wpAjaxUrl }
 						readerThemesEndpoint={ READER_THEMES_REST_ENDPOINT }
 						updatesNonce={ UPDATES_NONCE }

assets/src/setup/pages/choose-reader-theme/index.js

@@ -39,15 +39,9 @@ export function ChooseReaderTheme() {
 		}
 	}, [ canGoForward, setCanGoForward, readerTheme, themes, themeSupport ] );
 
-	// Filter out the active theme if it is a reader theme.
-	const nonActiveThemes = useMemo(
-		() => ( themes || [] ).filter( ( { availability } ) => 'active' !== availability ),
-		[ themes ],
-	);
-
 	// Separate available themes (both installed and installable) from those that need to be installed manually.
 	const { availableThemes, unavailableThemes } = useMemo(
-		() => nonActiveThemes.reduce(
+		() => themes.reduce(
 			( collections, theme ) => {
 				if ( theme.availability === 'non-installable' ) {
 					collections.unavailableThemes.push( theme );
@@ -59,7 +53,7 @@ export function ChooseReaderTheme() {
 			},
 			{ availableThemes: [], unavailableThemes: [] },
 		),
-		[ nonActiveThemes ] );
+		[ themes ] );
 
 	if ( fetchingThemes ) {
 		return (

assets/src/setup/pages/summary/index.js

@@ -4,17 +4,13 @@
 import { useContext, useEffect, useCallback } from '@wordpress/element';
 import { __ } from '@wordpress/i18n';
 
-/**
- * External dependencies
- */
-import { CURRENT_THEME } from 'amp-setup'; // From WP inline script.
-
 /**
  * Internal dependencies
  */
 import './style.css';
 import { Navigation } from '../../components/navigation-context-provider';
 import { Options } from '../../components/options-context-provider';
+import { ReaderThemes } from '../../components/reader-themes-context-provider';
 import { Reader } from './reader';
 import { Standard } from './standard';
 import { Transitional } from './transitional';
@@ -25,6 +21,7 @@ import { Transitional } from './transitional';
 export function Summary() {
 	const { setCanGoForward } = useContext( Navigation );
 	const { editedOptions } = useContext( Options );
+	const { currentTheme } = useContext( ReaderThemes );
 
 	const { theme_support: themeSupport } = editedOptions;
 
@@ -42,18 +39,18 @@ export function Summary() {
 
 		switch ( themeSupport ) {
 			case 'reader':
-				return <Reader currentTheme={ CURRENT_THEME } />;
+				return <Reader currentTheme={ currentTheme } />;
 
 			case 'standard':
-				return <Standard currentTheme={ CURRENT_THEME } />;
+				return <Standard currentTheme={ currentTheme } />;
 
 			case 'transitional':
-				return <Transitional currentTheme={ CURRENT_THEME } />;
+				return <Transitional currentTheme={ currentTheme } />;
 
 			default:
 				throw new Error( __( 'A mode option was not accounted for on the summary screen.', 'amp' ) );
 		}
-	}, [ themeSupport ] );
+	}, [ currentTheme, themeSupport ] );
 
 	return (
 		<div className="summary">

assets/src/setup/pages/summary/style.css

@@ -3,6 +3,10 @@
 	padding: 40px 30px;
 }
 
+.summary .amp-notice {
+	margin-bottom: 1.5rem;
+}
+
 /**
  * Reader-specific styles.
  */

assets/src/setup/pages/summary/transitional.js

@@ -11,7 +11,10 @@ import { __ } from '@wordpress/i18n';
 /**
  * Internal dependencies
  */
+import { useContext } from '@wordpress/element';
 import { Transitional as TransitionalIllustration } from '../../components/svg/transitional';
+import { ReaderThemes } from '../../components/reader-themes-context-provider';
+import { AMPNotice, NOTICE_TYPE_INFO, NOTICE_SIZE_LARGE } from '../../components/amp-notice';
 import { RedirectToggle } from './redirect-toggle';
 import { SummaryHeader } from './summary-header';
 import { DesktopScreenshot } from './desktop-screenshot';
@@ -23,8 +26,15 @@ import { DesktopScreenshot } from './desktop-screenshot';
  * @param {Object} props.currentTheme Data for the theme currently active on the site.
  */
 export function Transitional( { currentTheme } ) {
+	const { readerModeWasOverridden } = useContext( ReaderThemes );
+
 	return (
 		<>
+			{ readerModeWasOverridden && (
+				<AMPNotice type={ NOTICE_TYPE_INFO } size={ NOTICE_SIZE_LARGE }>
+					{ __( 'The reader theme you chose is the currently active theme on your site, which means reader mode provides no benefit. Your mode has been switched from reader to transitional.', 'amp' ) }
+				</AMPNotice>
+			) }
 			<SummaryHeader
 				illustration={ <TransitionalIllustration /> }
 				title={ __( 'Transitional', 'amp' ) }

assets/src/setup/pages/template-mode/get-selection-details.js

@@ -29,12 +29,13 @@ export const NON_TECHNICAL = 'nonTechnical';
  * Returns the degree to which each mode is recommended for the current site and user.
  *
  * @param {Object} args
+ * @param {boolean} args.currentThemeIsReaderTheme Whether the currently active theme is in the reader themes list.
  * @param {boolean} args.userIsTechnical Whether the user answered yes to the technical question.
  * @param {boolean} args.hasPluginIssues Whether the site scan found plugin issues.
  * @param {boolean} args.hasThemeIssues Whether the site scan found theme issues.
  * @param {boolean} args.hasScanResults Whether there are available scan results.
  */
-export function getRecommendationLevels( { userIsTechnical, hasPluginIssues, hasThemeIssues, hasScanResults = true } ) {
+export function getRecommendationLevels( { currentThemeIsReaderTheme, userIsTechnical, hasPluginIssues, hasThemeIssues, hasScanResults = true } ) {
 	// Handle case where scanning has failed or did not run.
 	if ( ! hasScanResults ) {
 		if ( userIsTechnical ) {
@@ -47,7 +48,7 @@ export function getRecommendationLevels( { userIsTechnical, hasPluginIssues, has
 		return {
 			[ READER ]: MOST_RECOMMENDED,
 			[ STANDARD ]: RECOMMENDED,
-			[ TRANSITIONAL ]: RECOMMENDED,
+			[ TRANSITIONAL ]: currentThemeIsReaderTheme ? MOST_RECOMMENDED : RECOMMENDED,
 		};
 	}