Boost: Update critical css errors UI (#37658)

* Update show stopper UI * Add error message when unknown error occurs during css generation * add changelog * Improve readability of show stopper component * Add retry to "css gen library missing" error
Automattic · Jun 6, 2024 · 1416be0 · 1416be0
1 parent d8770c5
commit 1416be0
Show file tree

Hide file tree

Showing 12 changed files with 270 additions and 86 deletions.
diff --git a/...ins/boost/app/assets/src/js/features/critical-css/error-description/error-description.tsx b/...ins/boost/app/assets/src/js/features/critical-css/error-description/error-description.tsx
@@ -1,29 +1,21 @@
 import classNames from 'classnames';
 import { createInterpolateElement } from '@wordpress/element';
 import { __ } from '@wordpress/i18n';
-import {
-	describeErrorSet,
-	suggestion,
-	rawError,
-} from '../lib/describe-critical-css-recommendations';
-import actionLinkInterpolateVar from '$lib/utils/action-link-interpolate-var';
-import { type InterpolateVars } from '$lib/utils/interplate-vars-types';
-import supportLinkInterpolateVar from '$lib/utils/support-link-interpolate-var';
+import { describeErrorSet, rawError } from '../lib/describe-critical-css-recommendations';
 import FoldingElement from '../folding-element/folding-element';
 import MoreList from '../more-list/more-list';
 import styles from './error-description.module.scss';
 import Suggestion from '../suggestion/suggestion';
 import { CriticalCssErrorDescriptionTypes, FormattedURL } from './types';
-import { useRegenerateCriticalCssAction } from '../lib/stores/critical-css-state';
-import { useNavigate } from 'react-router-dom';
+import getCriticalCssErrorSetInterpolateVars from '$lib/utils/get-critical-css-error-set-interpolate-vars';
 
 /**
  * Remove GET parameters that are used to cache-bust from display URLs, as they add visible noise
  * to the error output with no real benefit to users understanding which URLs are problematic.
  *
  * @param url The URL to strip cache parameters from.
  */
-function stripCacheParams( url: string ): string {
+export function stripCacheParams( url: string ): string {
 	const urlObj = new URL( url );
 	urlObj.searchParams.delete( 'donotcachepage' );
 	return urlObj.toString();
@@ -49,26 +41,7 @@ const CriticalCssErrorDescription: React.FC< CriticalCssErrorDescriptionTypes >
 	} );
 
 	const rawErrors = rawError( errorSet );
-	const regenerateAction = useRegenerateCriticalCssAction();
-	const navigate = useNavigate();
-
-	function retry() {
-		regenerateAction.mutate();
-		navigate( '/' );
-	}
-
-	const intepolateVars: InterpolateVars = {
-		...actionLinkInterpolateVar( retry, 'retry' ),
-		...supportLinkInterpolateVar(),
-		b: <b />,
-	};
-
-	if ( 'listLink' in suggestion( errorSet ) ) {
-		intepolateVars.link = (
-			// eslint-disable-next-line jsx-a11y/anchor-has-content
-			<a href={ suggestion( errorSet ).listLink } target="_blank" rel="noreferrer" />
-		);
-	}
+	const intepolateVars = getCriticalCssErrorSetInterpolateVars( errorSet );
 
 	return (
 		<div className={ styles[ 'error-description' ] }>

diff --git a/...boost/app/assets/src/js/features/critical-css/folding-element/folding-element.module.scss b/...boost/app/assets/src/js/features/critical-css/folding-element/folding-element.module.scss
@@ -2,7 +2,6 @@
 	&.foldable-element-control {
 		color: var( --primary-black );
 		font-size: 16px;
-		margin-bottom: 24px;
 	}
 }
 
@@ -27,6 +26,16 @@
 	animation-name: fadeIn;
 }
 
-.expanded {
-	margin-bottom: 24px;
+.expanded p {
+	padding-top: 1em;
+	padding-bottom: 1em;
+	margin: 0 !important;
+}
+
+.expanded p:last-child {
+	padding-bottom: 0;
+}
+
+.foldable-element-control {
+	font-weight: 600;
 }
diff --git a/...plugins/boost/app/assets/src/js/features/critical-css/folding-element/folding-element.tsx b/...plugins/boost/app/assets/src/js/features/critical-css/folding-element/folding-element.tsx
@@ -2,6 +2,8 @@ import useMeasure from 'react-use-measure';
 import { animated, useSpring } from '@react-spring/web';
 import classNames from 'classnames';
 import { useState } from 'react';
+import ChevronDown from '$svg/chevron-down';
+import ChevronUp from '$svg/chevron-up';
 import styles from './folding-element.module.scss';
 
 type PropTypes = {
@@ -34,6 +36,7 @@ const FoldingElement: React.FC< PropTypes > = ( {
 				onClick={ () => setExpanded( ! expanded ) }
 			>
 				{ label }
+				{ expanded ? <ChevronUp /> : <ChevronDown /> }
 			</button>
 
 			<animated.div

diff --git a/projects/plugins/boost/app/assets/src/js/features/critical-css/lib/generate-critical-css.ts b/projects/plugins/boost/app/assets/src/js/features/critical-css/lib/generate-critical-css.ts
@@ -191,7 +191,7 @@ async function generateForKeys(
 		CriticalCSSGeneratorSchema.parse( CriticalCSSGenerator );
 	} catch ( err ) {
 		recordBoostEvent( 'critical_css_library_failure', {} );
-		throw new Error( 'Critical CSS Generator library is either not found or invalid.' );
+		throw new Error( 'css-gen-library-failure' );
 	}
 
 	// eslint-disable-next-line @wordpress/no-unused-vars-before-return

diff --git a/...ins/boost/app/assets/src/js/features/critical-css/numbered-list/numbered-list.module.scss b/...ins/boost/app/assets/src/js/features/critical-css/numbered-list/numbered-list.module.scss
@@ -1,24 +1,31 @@
 .numbered-list {
 	list-style-type: none;
 	margin-left: 0;
+	margin: 0;
 
 	li {
-		margin-bottom: 0.5em;
+		margin-bottom: 1em;
 		display: flex;
 	}
 
+	li:last-child {
+		margin-bottom: 0;
+	}
+
 	.text {
-	  	flex: 1;
+		flex: 1;
+		line-height: 1.3em;
 	}
 
 	.index {
 		border: 1px solid #ccc;
 		border-radius: 50%;
 		display: block;
-		width: 1.5em;
-		height: 1.5em;
+		width: 1.4em;
+		height: 1.4em;
 		text-align: center;
-		line-height: 1.4em;
+		line-height: 1.3em;
+		font-size: 14px;
 		margin-right: 5px;
 	}
 }
diff --git a/...s/boost/app/assets/src/js/features/critical-css/show-stopper-error/show-stopper-error.tsx b/...s/boost/app/assets/src/js/features/critical-css/show-stopper-error/show-stopper-error.tsx
@@ -1,9 +1,15 @@
-import { __ } from '@wordpress/i18n';
-import ErrorNotice from '$features/error-notice/error-notice';
+import { __, sprintf } from '@wordpress/i18n';
 import FoldingElement from '../folding-element/folding-element';
-import ErrorDescription from '../error-description/error-description';
-import { getPrimaryErrorSet } from '../lib/critical-css-errors';
+import { ErrorSet, getPrimaryErrorSet } from '../lib/critical-css-errors';
 import { CriticalCssState } from '../lib/stores/critical-css-state-types';
+import { Notice } from '@automattic/jetpack-components';
+import { describeErrorSet, suggestion } from '../lib/describe-critical-css-recommendations';
+import { createInterpolateElement } from '@wordpress/element';
+import getSupportLinkCriticalCss from '$lib/utils/get-support-link-critical-css';
+import NumberedList from '../numbered-list/numbered-list';
+import getCriticalCssErrorSetInterpolateVars from '$lib/utils/get-critical-css-error-set-interpolate-vars';
+import formatErrorSetUrls from '$lib/utils/format-error-set-urls';
+import actionLinkInterpolateVar from '$lib/utils/action-link-interpolate-var';
 
 type ShowStopperErrorTypes = {
 	supportLink?: string;
@@ -19,9 +25,101 @@ const ShowStopperError: React.FC< ShowStopperErrorTypes > = ( {
 	showRetry,
 } ) => {
 	const primaryErrorSet = getPrimaryErrorSet( cssState );
-	const showErrorDescription = primaryErrorSet && cssState.status === 'generated';
-	const showFoldingElement = showErrorDescription || cssState.status_error;
+	const showLearnSection = primaryErrorSet && cssState.status === 'generated';
 
+	return (
+		<>
+			<Notice
+				level="error"
+				title={ __( 'Failed to generate Critical CSS', 'jetpack-boost' ) }
+				hideCloseButton={ true }
+			>
+				{ showLearnSection ? (
+					<>
+						<Description errorSet={ primaryErrorSet } />
+						<FoldingElement
+							labelExpandedText={ __( 'Learn what to do', 'jetpack-boost' ) }
+							labelCollapsedText={ __( 'Learn what to do', 'jetpack-boost' ) }
+						>
+							<div className="raw-error">
+								<p>{ __( 'Please follow the troubleshooting steps below', 'jetpack-boost' ) }</p>
+								<Steps errorSet={ primaryErrorSet } />
+								<DocumentationSection errorType={ primaryErrorSet.type.toString() } />
+							</div>
+						</FoldingElement>
+					</>
+				) : (
+					<OtherErrors
+						cssState={ cssState }
+						retry={ retry }
+						showRetry={ showRetry }
+						supportLink={ supportLink }
+					/>
+				) }
+			</Notice>
+		</>
+	);
+};
+
+const Description = ( { errorSet }: { errorSet: ErrorSet } ) => {
+	const displayUrls = formatErrorSetUrls( errorSet );
+
+	return (
+		<p>
+			{ createInterpolateElement( describeErrorSet( errorSet ), {
+				b: <b />,
+			} ) }{ ' ' }
+			{ displayUrls.map( ( { href, label }, index ) => (
+				<a href={ href } target="_blank" rel="noreferrer" key={ index }>
+					{ label }
+				</a>
+			) ) }
+		</p>
+	);
+};
+
+const Steps = ( { errorSet }: { errorSet: ErrorSet } ) => {
+	const details = suggestion( errorSet );
+	if ( ! details.list ) {
+		return null;
+	}
+
+	const interpolateVars = getCriticalCssErrorSetInterpolateVars( errorSet );
+
+	return <NumberedList items={ details.list } interpolateVars={ interpolateVars } />;
+};
+
+const DocumentationSection = ( {
+	message,
+	errorType,
+}: {
+	message?: string;
+	errorType: string;
+} ) => {
+	if ( message === undefined ) {
+		message = __(
+			'If you are still experiencing this issue, <link>learn more</link> from our documentation.',
+			'jetpack-boost'
+		);
+	}
+
+	return (
+		<p>
+			{ createInterpolateElement( message, {
+				link: (
+					// eslint-disable-next-line jsx-a11y/anchor-has-content
+					<a
+						href={ getSupportLinkCriticalCss( errorType ) }
+						target="_blank"
+						rel="noopener noreferrer"
+					/>
+				),
+			} ) }
+		</p>
+	);
+};
+
+const OtherErrors = ( { cssState, retry, showRetry, supportLink }: ShowStopperErrorTypes ) => {
 	const firstTimeError = __(
 		'An unexpected error has occurred. As this error may be temporary, please try and refresh the Critical CSS.',
 		'jetpack-boost'
@@ -33,47 +131,61 @@ const ShowStopperError: React.FC< ShowStopperErrorTypes > = ( {
 	);
 
 	return (
-		<ErrorNotice
-			title={ __( 'Failed to generate Critical CSS', 'jetpack-boost' ) }
-			variant="module"
-			actionButton={
-				showRetry ? (
-					<button className="secondary" onClick={ retry }>
-						{ __( 'Refresh', 'jetpack-boost' ) }
-					</button>
-				) : (
-					<a
-						className="button button-secondary"
-						href={ supportLink }
-						target="_blank"
-						rel="noreferrer"
-					>
-						{ __( 'Contact Support', 'jetpack-boost' ) }
-					</a>
-				)
-			}
-		>
-			<p>{ showRetry ? firstTimeError : secondTimeError }</p>
-			{ showFoldingElement && (
-				<FoldingElement
-					labelExpandedText={ __( 'See error message', 'jetpack-boost' ) }
-					labelCollapsedText={ __( 'Hide error message', 'jetpack-boost' ) }
-				>
-					<div className="raw-error">
-						{ showErrorDescription ? (
-							<ErrorDescription
-								errorSet={ primaryErrorSet }
-								showSuggestion={ true }
-								showClosingParagraph={ false }
-								foldRawErrors={ false }
-							/>
-						) : (
+		<>
+			{ cssState.status_error === 'css-gen-library-failure' ? (
+				<>
+					<p>
+						{ __(
+							'Critical CSS Generator library is either not found or invalid.',
+							'jetpack-boost'
+						) }
+					</p>
+					<DocumentationSection
+						message={ __(
+							'<link>Learn how</link> to fix this by visiting our documentation.',
+							'jetpack-boost'
+						) }
+						errorType={ cssState.status_error }
+					/>
+					<p>
+						{ createInterpolateElement(
+							__(
+								'If the problem has been resolved, refresh the page and click <retry>here</retry> to try regenerating critical css.',
+								'jetpack-boost'
+							),
+							{
+								...actionLinkInterpolateVar( retry, 'retry' ),
+							}
+						) }
+					</p>
+				</>
+			) : (
+				<>
+					<p>{ showRetry ? firstTimeError : secondTimeError }</p>
+					<p>
+						{ sprintf(
+							/* translators: %s: error message */
+							__( `Error: %s`, 'jetpack-boost' ),
 							cssState.status_error
 						) }
-					</div>
-				</FoldingElement>
+					</p>
+					{ showRetry ? (
+						<button className="secondary" onClick={ retry }>
+							{ __( 'Refresh', 'jetpack-boost' ) }
+						</button>
+					) : (
+						<a
+							className="button button-secondary"
+							href={ supportLink }
+							target="_blank"
+							rel="noreferrer"
+						>
+							{ __( 'Contact Support', 'jetpack-boost' ) }
+						</a>
+					) }
+				</>
 			) }
-		</ErrorNotice>
+		</>
 	);
 };
 

diff --git a/projects/plugins/boost/app/assets/src/js/lib/utils/format-error-set-urls.ts b/projects/plugins/boost/app/assets/src/js/lib/utils/format-error-set-urls.ts
@@ -0,0 +1,18 @@
+import { stripCacheParams } from '$features/critical-css/error-description/error-description';
+import { FormattedURL } from '$features/critical-css/error-description/types';
+import { ErrorSet } from '$features/critical-css/lib/critical-css-errors';
+
+function formatErrorSetUrls( errorSet: ErrorSet ): FormattedURL[] {
+	return Object.entries( errorSet.byUrl ).map( ( [ url, error ] ) => {
+		let href = url;
+		if ( error.meta.url && typeof error.meta.url === 'string' ) {
+			href = error.meta.url;
+		}
+		return {
+			href,
+			label: stripCacheParams( url ),
+		};
+	} );
+}
+
+export default formatErrorSetUrls;