Skip to content

Commit

Permalink
Refactor template mode recommendation logic; use it on AMP Settings
Browse files Browse the repository at this point in the history
  • Loading branch information
delawski committed Oct 27, 2021
1 parent 7d2f1d1 commit cf218ea
Show file tree
Hide file tree
Showing 11 changed files with 141 additions and 172 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import { READER, STANDARD, TRANSITIONAL } from '../../../common/constants';
import { READER, STANDARD, TRANSITIONAL } from '../constants';

// Recommendation levels.
export const RECOMMENDED = 'recommended';
Expand All @@ -23,14 +24,20 @@ export const NON_TECHNICAL = 'nonTechnical';
*
* @param {Object} args
* @param {boolean} args.currentThemeIsAmongReaderThemes Whether the currently active theme is in the reader themes list.
* @param {boolean} args.hasPluginsWithAmpIncompatibility Whether the site scan found plugins with AMP incompatibility.
* @param {boolean} args.hasSiteScanResults Whether there are available site scan results.
* @param {boolean} args.hasThemesWithAmpIncompatibility Whether the site scan found themes with AMP incompatibility.
* @param {boolean} args.userIsTechnical Whether the user answered yes to the technical question.
* @param {boolean} args.hasPluginsWithAMPIncompatibility Whether the site scan found plugins with AMP incompatibility.
* @param {boolean} args.hasThemesWithAMPIncompatibility Whether the site scan found themes with AMP incompatibility.
* @param {boolean} args.hasScanResults Whether there are available scan results.
*/
export function getSelectionDetails( { currentThemeIsAmongReaderThemes, userIsTechnical, hasPluginsWithAMPIncompatibility, hasThemesWithAMPIncompatibility, hasScanResults = true } ) {
export function getTemplateModeRecommendation( {
currentThemeIsAmongReaderThemes,
hasPluginsWithAmpIncompatibility,
hasSiteScanResults,
hasThemesWithAmpIncompatibility,
userIsTechnical,
} ) {
// Handle case where scanning has failed or did not run.
if ( ! hasScanResults ) {
if ( ! hasSiteScanResults ) {
return {
[ READER ]: {
recommendationLevel: ( userIsTechnical || currentThemeIsAmongReaderThemes ) ? RECOMMENDED : NEUTRAL,
Expand All @@ -54,9 +61,9 @@ export function getSelectionDetails( { currentThemeIsAmongReaderThemes, userIsTe
}

switch ( true ) {
case hasThemesWithAMPIncompatibility && hasPluginsWithAMPIncompatibility && userIsTechnical:
case hasThemesWithAMPIncompatibility && ! hasPluginsWithAMPIncompatibility && userIsTechnical:
case ! hasThemesWithAMPIncompatibility && hasPluginsWithAMPIncompatibility && userIsTechnical:
case hasThemesWithAmpIncompatibility && hasPluginsWithAmpIncompatibility && userIsTechnical:
case hasThemesWithAmpIncompatibility && ! hasPluginsWithAmpIncompatibility && userIsTechnical:
case ! hasThemesWithAmpIncompatibility && hasPluginsWithAmpIncompatibility && userIsTechnical:
return {
[ READER ]: {
recommendationLevel: NEUTRAL,
Expand All @@ -81,8 +88,8 @@ export function getSelectionDetails( { currentThemeIsAmongReaderThemes, userIsTe
},
};

case hasThemesWithAMPIncompatibility && hasPluginsWithAMPIncompatibility && ! userIsTechnical:
case ! hasThemesWithAMPIncompatibility && hasPluginsWithAMPIncompatibility && ! userIsTechnical:
case hasThemesWithAmpIncompatibility && hasPluginsWithAmpIncompatibility && ! userIsTechnical:
case ! hasThemesWithAmpIncompatibility && hasPluginsWithAmpIncompatibility && ! userIsTechnical:
return {
[ READER ]: {
recommendationLevel: RECOMMENDED,
Expand All @@ -105,7 +112,7 @@ export function getSelectionDetails( { currentThemeIsAmongReaderThemes, userIsTe
},
};

case hasThemesWithAMPIncompatibility && ! hasPluginsWithAMPIncompatibility && ! userIsTechnical:
case hasThemesWithAmpIncompatibility && ! hasPluginsWithAmpIncompatibility && ! userIsTechnical:
return {
[ READER ]: {
recommendationLevel: RECOMMENDED,
Expand All @@ -129,7 +136,7 @@ export function getSelectionDetails( { currentThemeIsAmongReaderThemes, userIsTe
},
};

case ! hasThemesWithAMPIncompatibility && ! hasPluginsWithAMPIncompatibility && userIsTechnical:
case ! hasThemesWithAmpIncompatibility && ! hasPluginsWithAmpIncompatibility && userIsTechnical:
return {
[ READER ]: {
recommendationLevel: NOT_RECOMMENDED,
Expand All @@ -151,7 +158,7 @@ export function getSelectionDetails( { currentThemeIsAmongReaderThemes, userIsTe
},
};

case ! hasThemesWithAMPIncompatibility && ! hasPluginsWithAMPIncompatibility && ! userIsTechnical:
case ! hasThemesWithAmpIncompatibility && ! hasPluginsWithAmpIncompatibility && ! userIsTechnical:
return {
[ READER ]: {
recommendationLevel: NOT_RECOMMENDED,
Expand Down
17 changes: 17 additions & 0 deletions assets/src/common/helpers/test/get-template-mode-recommendation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* Internal dependencies
*/
import { getTemplateModeRecommendation } from '../get-template-mode-recommendation';

describe( 'getTemplateModeRecommendation', () => {
it( 'throws no errors', () => {
[ true, false ].forEach( ( hasPluginsWithAmpIncompatibility ) => {
[ true, false ].forEach( ( hasThemesWithAmpIncompatibility ) => {
[ true, false ].forEach( ( userIsTechnical ) => {
const cb = () => getTemplateModeRecommendation( { hasPluginsWithAmpIncompatibility, hasThemesWithAmpIncompatibility, userIsTechnical } );
expect( cb ).not.toThrow();
} );
} );
} );
} );
} );
2 changes: 1 addition & 1 deletion assets/src/components/site-scan-context-provider/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ export function SiteScanContextProvider( {
isCancelled: status === STATUS_CANCELLED,
isCompleted: status === STATUS_COMPLETED,
isFailed: status === STATUS_FAILED,
isInitializing: [ STATUS_REQUEST_SCANNABLE_URLS, STATUS_FETCHING_SCANNABLE_URLS ].includes( status ),
isFetchingScannableUrls: [ STATUS_REQUEST_SCANNABLE_URLS, STATUS_FETCHING_SCANNABLE_URLS ].includes( status ),
isReady: status === STATUS_READY,
isSiteScannable: scannableUrls.length > 0,
pluginsWithAmpIncompatibility,
Expand Down
6 changes: 3 additions & 3 deletions assets/src/components/template-mode-option/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ export function TemplateModeOption( { children, details, detailsUrl, initialOpen
selected={ mode === themeSupport }
>
<div className="template-mode-selection__details">
{ children }
{ Array.isArray( details ) && (
<ul className="template-mode-selection__details-list">
{ details.map( ( detail, index ) => (
Expand All @@ -148,7 +149,7 @@ export function TemplateModeOption( { children, details, detailsUrl, initialOpen
) ) }
</ul>
) }
{ ! Array.isArray( details ) && (
{ details && ! Array.isArray( details ) && (
<p>
<span dangerouslySetInnerHTML={ { __html: details } } />
{ detailsUrl && (
Expand All @@ -161,7 +162,6 @@ export function TemplateModeOption( { children, details, detailsUrl, initialOpen
) }
</p>
) }
{ children }
</div>
</AMPDrawer>
);
Expand All @@ -172,7 +172,7 @@ TemplateModeOption.propTypes = {
details: PropTypes.oneOfType( [
PropTypes.string,
PropTypes.arrayOf( PropTypes.string ),
] ).isRequired,
] ),
detailsUrl: PropTypes.string,
initialOpen: PropTypes.bool,
labelExtra: PropTypes.node,
Expand Down
12 changes: 6 additions & 6 deletions assets/src/onboarding-wizard/pages/site-scan/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export function SiteScan() {
isCancelled,
isCompleted,
isFailed,
isInitializing,
isFetchingScannableUrls,
isReady,
pluginsWithAmpIncompatibility,
scannableUrls,
Expand Down Expand Up @@ -71,7 +71,7 @@ export function SiteScan() {
*/
const isDelayedCompleted = useDelayedFlag( isCompleted );

if ( isInitializing ) {
if ( isFetchingScannableUrls ) {
return (
<SiteScanPanel
title={ __( 'Please wait a minute…', 'amp' ) }
Expand Down Expand Up @@ -115,22 +115,22 @@ export function SiteScan() {
<ThemesWithAmpIncompatibility
className="site-scan__section"
slugs={ themesWithAmpIncompatibility }
callToAction={ userIsTechnical && (
callToAction={ userIsTechnical ? (
<ExternalLink href={ VALIDATED_URLS_LINK }>
{ __( 'Review Validated URLs', 'amp' ) }
</ExternalLink>
) }
) : null }
/>
) }
{ pluginsWithAmpIncompatibility.length > 0 && (
<PluginsWithAmpIncompatibility
className="site-scan__section"
slugs={ pluginsWithAmpIncompatibility }
callToAction={ userIsTechnical && (
callToAction={ userIsTechnical ? (
<ExternalLink href={ VALIDATED_URLS_LINK }>
{ __( 'Review Validated URLs', 'amp' ) }
</ExternalLink>
) }
) : null }
/>
) }
</SiteScanPanel>
Expand Down
11 changes: 5 additions & 6 deletions assets/src/onboarding-wizard/pages/template-mode/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,12 @@ import { ScreenUI } from './screen-ui';
*/
export function TemplateMode() {
const { setCanGoForward } = useContext( Navigation );
const { editedOptions, originalOptions, updateOptions } = useContext( Options );
const { editedOptions: { theme_support: themeSupport }, originalOptions, updateOptions } = useContext( Options );
const { developerToolsOption } = useContext( User );
const { pluginsWithAmpIncompatibility, themesWithAmpIncompatibility } = useContext( SiteScan );
const { currentTheme } = useContext( ReaderThemes );
const { hasSiteScanResults, pluginsWithAmpIncompatibility, themesWithAmpIncompatibility } = useContext( SiteScan );
const { currentTheme: { is_reader_theme: currentThemeIsAmongReaderThemes } } = useContext( ReaderThemes );
const { technicalQuestionChangedAtLeastOnce } = useContext( TemplateModeOverride );

const { theme_support: themeSupport } = editedOptions;

/**
* Allow moving forward.
*/
Expand Down Expand Up @@ -57,9 +55,10 @@ export function TemplateMode() {
</div>
<ScreenUI
currentMode={ themeSupport }
currentThemeIsAmongReaderThemes={ currentTheme.is_reader_theme }
currentThemeIsAmongReaderThemes={ currentThemeIsAmongReaderThemes }
developerToolsOption={ developerToolsOption }
firstTimeInWizard={ false === originalOptions.plugin_configured }
hasSiteScanResults={ hasSiteScanResults }
pluginsWithAmpIncompatibility={ pluginsWithAmpIncompatibility }
savedCurrentMode={ originalOptions.theme_support }
setCurrentMode={ ( mode ) => {
Expand Down
41 changes: 21 additions & 20 deletions assets/src/onboarding-wizard/pages/template-mode/screen-ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import { READER, STANDARD, TRANSITIONAL } from '../../../common/constants';
import {
RECOMMENDED,
NOT_RECOMMENDED,
getSelectionDetails,
} from './get-selection-details';
getTemplateModeRecommendation,
} from '../../../common/helpers/get-template-mode-recommendation';

/**
* Small notice indicating a mode is recommended.
Expand Down Expand Up @@ -72,6 +72,7 @@ function isInitiallyOpen( mode, selectionDetails, savedCurrentMode ) {
* @param {boolean} props.currentThemeIsAmongReaderThemes Whether the currently active theme is in the list of reader themes.
* @param {boolean} props.developerToolsOption Whether the user has enabled developer tools.
* @param {boolean} props.firstTimeInWizard Whether the wizard is running for the first time.
* @param {boolean} props.hasSiteScanResults Whether there are available site scan results.
* @param {boolean} props.technicalQuestionChanged Whether the user changed their technical question from the previous option.
* @param {string[]} props.pluginsWithAmpIncompatibility A list of plugin slugs causing AMP incompatibility.
* @param {string} props.savedCurrentMode The current selected mode saved in the database.
Expand All @@ -81,47 +82,46 @@ export function ScreenUI( {
currentThemeIsAmongReaderThemes,
developerToolsOption,
firstTimeInWizard,
technicalQuestionChanged,
hasSiteScanResults,
pluginsWithAmpIncompatibility,
savedCurrentMode,
technicalQuestionChanged,
themesWithAmpIncompatibility,
} ) {
const userIsTechnical = useMemo( () => developerToolsOption === true, [ developerToolsOption ] );

const selectionDetails = useMemo( () => getSelectionDetails(
const templateModeRecommendation = useMemo( () => getTemplateModeRecommendation(
{
currentThemeIsAmongReaderThemes,
userIsTechnical,
hasScanResults: null !== pluginsWithAmpIncompatibility && null !== themesWithAmpIncompatibility,
hasPluginsWithAMPIncompatibility: pluginsWithAmpIncompatibility && 0 < pluginsWithAmpIncompatibility.length,
hasThemesWithAMPIncompatibility: themesWithAmpIncompatibility && 0 < themesWithAmpIncompatibility.length,
hasPluginsWithAmpIncompatibility: pluginsWithAmpIncompatibility?.length > 0,
hasSiteScanResults,
hasThemesWithAmpIncompatibility: themesWithAmpIncompatibility?.length > 0,
userIsTechnical: developerToolsOption === true,
},
), [ currentThemeIsAmongReaderThemes, themesWithAmpIncompatibility, pluginsWithAmpIncompatibility, userIsTechnical ] );
), [ currentThemeIsAmongReaderThemes, developerToolsOption, hasSiteScanResults, pluginsWithAmpIncompatibility, themesWithAmpIncompatibility ] );

return (
<form>
<TemplateModeOption
details={ selectionDetails[ READER ].details }
initialOpen={ isInitiallyOpen( READER, selectionDetails, savedCurrentMode ) }
details={ templateModeRecommendation[ READER ].details }
initialOpen={ isInitiallyOpen( READER, templateModeRecommendation, savedCurrentMode ) }
mode={ READER }
previouslySelected={ savedCurrentMode === READER && technicalQuestionChanged && ! firstTimeInWizard }
labelExtra={ selectionDetails[ READER ].recommendationLevel === RECOMMENDED ? <RecommendedNotice /> : null }
labelExtra={ templateModeRecommendation[ READER ].recommendationLevel === RECOMMENDED ? <RecommendedNotice /> : null }
/>

<TemplateModeOption
details={ selectionDetails[ TRANSITIONAL ].details }
initialOpen={ isInitiallyOpen( TRANSITIONAL, selectionDetails, savedCurrentMode ) }
details={ templateModeRecommendation[ TRANSITIONAL ].details }
initialOpen={ isInitiallyOpen( TRANSITIONAL, templateModeRecommendation, savedCurrentMode ) }
mode={ TRANSITIONAL }
previouslySelected={ savedCurrentMode === TRANSITIONAL && technicalQuestionChanged && ! firstTimeInWizard }
labelExtra={ selectionDetails[ TRANSITIONAL ].recommendationLevel === RECOMMENDED ? <RecommendedNotice /> : null }
labelExtra={ templateModeRecommendation[ TRANSITIONAL ].recommendationLevel === RECOMMENDED ? <RecommendedNotice /> : null }
/>

<TemplateModeOption
details={ selectionDetails[ STANDARD ].details }
initialOpen={ isInitiallyOpen( STANDARD, selectionDetails, savedCurrentMode ) }
details={ templateModeRecommendation[ STANDARD ].details }
initialOpen={ isInitiallyOpen( STANDARD, templateModeRecommendation, savedCurrentMode ) }
mode={ STANDARD }
previouslySelected={ savedCurrentMode === STANDARD && technicalQuestionChanged && ! firstTimeInWizard }
labelExtra={ selectionDetails[ STANDARD ].recommendationLevel === RECOMMENDED ? <RecommendedNotice /> : null }
labelExtra={ templateModeRecommendation[ STANDARD ].recommendationLevel === RECOMMENDED ? <RecommendedNotice /> : null }
/>
</form>
);
Expand All @@ -131,6 +131,7 @@ ScreenUI.propTypes = {
currentThemeIsAmongReaderThemes: PropTypes.bool.isRequired,
developerToolsOption: PropTypes.bool,
firstTimeInWizard: PropTypes.bool,
hasSiteScanResults: PropTypes.bool,
technicalQuestionChanged: PropTypes.bool,
pluginsWithAmpIncompatibility: PropTypes.arrayOf( PropTypes.string ),
savedCurrentMode: PropTypes.string,
Expand Down

This file was deleted.

6 changes: 3 additions & 3 deletions assets/src/settings-page/site-scan.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export function SiteScan( { onSiteScan } ) {
isCancelled,
isCompleted,
isFailed,
isInitializing,
isFetchingScannableUrls,
isReady,
isSiteScannable,
pluginsWithAmpIncompatibility,
Expand Down Expand Up @@ -84,7 +84,7 @@ export function SiteScan( { onSiteScan } ) {
* Get main content.
*/
const getContent = useCallback( () => {
if ( isInitializing ) {
if ( isFetchingScannableUrls ) {
return <Loading />;
}

Expand Down Expand Up @@ -123,7 +123,7 @@ export function SiteScan( { onSiteScan } ) {
}

return <SiteScanInProgress />;
}, [ isCancelled, isFailed, isInitializing, isSiteScannable, isSummary ] );
}, [ isCancelled, isFailed, isFetchingScannableUrls, isSiteScannable, isSummary ] );

return (
<SiteScanDrawer
Expand Down
Loading

0 comments on commit cf218ea

Please sign in to comment.