Merge pull request #1129 from Yoast/P2-704-title-block

Added title block instruction.

packages/schema-blocks/src/core/validation/BlockValidationResult.ts

@@ -1,5 +1,6 @@
 import { BlockValidation } from ".";
 import { BlockInstance } from "@wordpress/blocks";
+import { __, sprintf } from "@wordpress/i18n";
 
 export enum BlockPresence {
 	Required = "required",
@@ -35,20 +36,28 @@ export class BlockValidationResult {
 	/**
 	 * The validation issues for this block's innerblocks or attributes, if any.
 	 */
-	public issues: BlockValidationResult[]
+	public issues: BlockValidationResult[];
 
 	/**
-	 * @param clientId  The clientId of the validated block.
-	 * @param name      The name of the validated block.
-	 * @param result    The validation result.
+	 * An optional message describing the result.
+	 */
+	public message: string;
+
+	/**
+	 * @param clientId      The clientId of the validated block.
+	 * @param name          The name of the validated block.
+	 * @param result        The validation result.
 	 * @param blockPresence The block type.
+	 * @param message       An optional message describing the result.
 	 */
-	constructor( clientId: string, name: string, result: BlockValidation, blockPresence: BlockPresence ) {
+	constructor( clientId: string, name: string, result: BlockValidation, blockPresence: BlockPresence, message?: string ) {
+		this.name = name;
 		this.clientId = clientId;
 		this.name = name;
 		this.result = result;
 		this.blockPresence = blockPresence;
 		this.issues = [];
+		this.message = message;
 	}
 
 	/**
@@ -68,6 +77,29 @@ export class BlockValidationResult {
 		);
 	}
 
+	/**
+	 * Named constructor for a 'missing block' validation result.
+	 *
+	 * @param name The name of the missing block.
+	 * @param blockPresence The block type.
+	 *
+	 * @constructor
+	 */
+	static MissingBlock( name: string, blockPresence: BlockPresence ) {
+		return new BlockValidationResult(
+			null,
+			name,
+			BlockValidation.MissingBlock,
+			blockPresence,
+			sprintf(
+				/* Translators: %1$s expands to the block name, %2$s expands to 'required' or 'recommended'. */
+				__( "The '%1$s' block is %2$s but missing.", "yoast-schema-blocks" ),
+				name,
+				blockPresence,
+			),
+		);
+	}
+
 	/**
 	 * Named constructor for a 'valid' validation result.
 	 *

packages/schema-blocks/src/functions/BlockHelper.ts

@@ -43,4 +43,25 @@ function getBlockType( blockName: string ): Block | undefined {
 	return select( "core/blocks" ).getBlockType( blockName );
 }
 
-export { getBlockByClientId, removeBlock, restoreBlock, getBlockType };
+/**
+ * Retrieves a human readable block name.
+ *
+ * @param blockName The block name (e.g. the Gutenberg block id).
+ *
+ * @returns A human readable block title.
+ */
+function getHumanReadableBlockName( blockName: string ): string {
+	const blockType = getBlockType( blockName ) || null;
+	if ( blockType ) {
+		return blockType.title;
+	}
+
+	const lastSlash = blockName.lastIndexOf( "/" );
+	if ( lastSlash < 0 || lastSlash === blockName.length - 1 ) {
+		return blockName;
+	}
+
+	return blockName.substring( lastSlash + 1 ).toLocaleLowerCase();
+}
+
+export { getBlockByClientId, removeBlock, restoreBlock, getBlockType, getHumanReadableBlockName };

packages/schema-blocks/src/functions/presenters/InnerBlocksSidebarPresenter.tsx

@@ -3,7 +3,7 @@ import { createElement } from "@wordpress/element";
 import { BlockInstance } from "@wordpress/blocks";
 import BlockSuggestions from "../../blocks/BlockSuggestions";
 import { __ } from "@wordpress/i18n";
-import getWarnings, { sidebarWarning } from "./SidebarWarningPresenter";
+import getWarnings, { SidebarWarning } from "./SidebarWarningPresenter";
 import { InnerBlocksInstructionOptions } from "../../instructions/blocks/InnerBlocksInstructionOptions";
 import { SvgIcon } from "@yoast/components";
 
@@ -41,7 +41,7 @@ export function innerBlocksSidebar( currentBlock: BlockInstance, options: InnerB
  *
  * @returns {ReactElement} A ReactElement containing the list of warnings.
  */
-function createWarningList( warnings: sidebarWarning[] ): ReactElement {
+function createWarningList( warnings: SidebarWarning[] ): ReactElement {
 	return (
 		<div className="yoast-block-sidebar-warnings">
 			<div className="yoast-block-sidebar-title">{ __( "Analysis", "yoast-schema-blocks" ) }</div>

packages/schema-blocks/src/functions/presenters/SidebarWarningPresenter.ts

@@ -1,28 +1,24 @@
-import { getBlockType } from "../BlockHelper";
 import { select } from "@wordpress/data";
-import { __ } from "@wordpress/i18n";
-import { get } from "lodash";
-import { BlockValidationResult } from "../../core/validation";
-import { BlockValidation } from "../../core/validation";
-import { BlockPresence } from "../../core/validation/BlockValidationResult";
+import { __, sprintf } from "@wordpress/i18n";
 
-const analysisMessageTemplates: Record<number, string> = {
-	[ BlockValidation.MissingBlock ]: "The '{child}' block is {presence} but missing.",
-	[ BlockValidation.MissingAttribute ]: "The '{child}' block is empty.",
-};
+import { BlockValidation, BlockValidationResult } from "../../core/validation";
+import { getHumanReadableBlockName } from "../BlockHelper";
+import { BlockPresence } from "../../core/validation/BlockValidationResult";
 
 type clientIdValidation = Record<string, BlockValidationResult>;
 
-type analysisIssue = {
-	name: string;
-	parent: string;
-	result: BlockValidation;
-	presence: string;
-};
-
-export type sidebarWarning = {
+/**
+ * A warning message for in the sidebar schema analysis.
+ */
+export type SidebarWarning = {
+	/**
+	 * The warning message.
+	 */
 	text: string;
-	color: string;
+	/**
+	 * Color of the warning.
+	 */
+	color: "red" | "orange" | "green";
 }
 
 /**
@@ -42,51 +38,41 @@ function getValidationResult( clientId: string ): BlockValidationResult | null {
 }
 
 /**
- * Transforms a template into a warning message given validation details.
+ * If some required blocks are missing.
  *
- * @param issue Details about the current issue.
+ * @param issues The block validation issues to check.
  *
- * @returns {string} The presentable warning message appropriate for this issue.
+ * @return `true` if some required blocks are missing, `false` if not.
  */
-export function replaceVariables( issue: analysisIssue ): string {
-	const warning = get( analysisMessageTemplates, issue.result, "" );
-	return warning.replace( "{parent}", __( issue.parent, "wpseo-schema-blocks" ) )
-		.replace( "{child}", __( issue.name, "wpseo-schema-blocks" ) )
-		.replace( "{presence}", __( issue.presence, "wpseo-schema-blocks" ) );
+function someMissingRequiredBlocks( issues: BlockValidationResult[] ) {
+	return issues.some( issue => issue.result === BlockValidation.MissingBlock && issue.blockPresence === BlockPresence.Required );
 }
 
 /**
  * Adds analysis conclusions to the footer.
  *
  * @param validation The validation result for the current block.
- * @param issues     The detected issues with metadata.
+ * @param issues     The detected issues.
  *
- * @returns {sidebarWarning} Any analysis conclusions that should be in the footer.
+ * @returns Any analysis conclusions that should be in the footer.
  */
-function getAnalysisConclusion( validation: BlockValidation, issues: analysisIssue[] ): sidebarWarning {
+function getAnalysisConclusion( validation: BlockValidationResult, issues: BlockValidationResult[] ): SidebarWarning {
+	let conclusionText = "";
+
 	// Show a red bullet when not all required blocks have been completed.
-	if ( issues.some( issue => issue.result === BlockValidation.MissingBlock && issue.presence === BlockPresence.Required ||
-		issue.result === BlockValidation.MissingAttribute ) ) {
-		return {
-			text: __( "Not all required blocks have been completed! No " + issues[ 0 ].parent +
-				" schema will be generated for your page.", "wpseo-schema-blocks" ),
-			color: "red",
-		} as sidebarWarning;
+	if ( someMissingRequiredBlocks( issues ) ) {
+		conclusionText = sprintf(
+			/* translators: %s expands to the schema block name. */
+			__( "Not all required blocks have been completed! No %s schema will be generated for your page.", "yoast-schema-blocks" ),
+			sanitizeParentName( getHumanReadableBlockName( validation.name ) ),
+		);
+
+		return { text: conclusionText, color: "red" };
 	}
 
-	// Show a green bullet when all required blocks have been completed.
-	const requiredBlockIssues = issues.filter( issue => {
-		return issue.presence === BlockPresence.Required;
-	} );
+	conclusionText = __( "Good job! All required blocks have been completed.", "yoast-schema-blocks" );
 
-	if ( validation === BlockValidation.Valid ||
-		requiredBlockIssues.every( issue => issue.result !== BlockValidation.MissingAttribute &&
-			issue.result !== BlockValidation.MissingBlock ) ) {
-		return {
-			text: __( "Good job! All required blocks have been completed.", "wpseo-schema-blocks" ),
-			color: "green",
-		} as sidebarWarning;
-	}
+	return { text: conclusionText, color: "green" };
 }
 
 /**
@@ -105,57 +91,55 @@ function getAllDescendantIssues( validation: BlockValidationResult ): BlockValid
 }
 
 /**
- * Creates an array of warning messages from a block validation result.
+ * Get a list of (red) error messages.
  *
- * @param validation The block being validated.
+ * @param issues The block validation issues.
  *
- * @returns {sidebarWarning[]} The formatted warnings.
+ * @return The error messages.
  */
-export function createAnalysisMessages( validation: BlockValidationResult ): sidebarWarning[] {
-	const parent = sanitizeBlockName( validation.name );
-
-	// Create a message if there are any validation issues we have a template for.
-	const messageData: analysisIssue[] = getAllDescendantIssues( validation )
-		.filter( ( issue: BlockValidationResult ) => issue.result in analysisMessageTemplates )
-		.map( ( issue: BlockValidationResult ) => ( {
-			name: sanitizeBlockName( issue.name ),
-			parent: sanitizeParentName( parent ),
-			result: issue.result,
-			presence: issue.blockPresence,
-		} ) );
-
-	const messages = messageData.map( msg => {
-		return { text: replaceVariables( msg ), color: ( msg.presence === BlockPresence.Recommended ? "orange" : "red" ) } as sidebarWarning;
-	} );
+function getErrorMessages( issues: BlockValidationResult[] ): SidebarWarning[] {
+	const requiredBlockIssues = issues.filter( issue => issue.message && issue.blockPresence === BlockPresence.Required );
 
-	const conclusion = getAnalysisConclusion( validation.result, messageData );
+	return requiredBlockIssues.map( issue => ( {
+		color: "red",
+		text: issue.message,
+	} ) );
+}
 
-	if ( conclusion ) {
-		messages.push( conclusion );
-	}
+/**
+ * Get a list of (orange) warning messages.
+ *
+ * @param issues The block validation issues.
+ *
+ * @return The warning messages.
+ */
+function getWarningMessages( issues: BlockValidationResult[] ): SidebarWarning[] {
+	const recommendedBlockIssues = issues.filter( issue => issue.message && issue.blockPresence === BlockPresence.Recommended );
 
-	return messages;
+	return recommendedBlockIssues.map( issue => ( {
+		color: "orange",
+		text: issue.message,
+	} ) );
 }
 
 /**
- * Makes a block name human readable.
+ * Creates an array of warning messages from a block validation result.
  *
- * @param blockName The block name to sanitize.
+ * @param validation The block being validated.
  *
- * @returns {string} The sanitized block name.
+ * @returns {SidebarWarning[]} The formatted warnings.
  */
-export function sanitizeBlockName( blockName: string ): string {
-	const blockType = getBlockType( blockName ) || "";
-	if ( blockType ) {
-		return blockType.title;
-	}
+export function createAnalysisMessages( validation: BlockValidationResult ): SidebarWarning[] {
+	const issues = getAllDescendantIssues( validation );
 
-	const lastSlash = blockName.lastIndexOf( "/" );
-	if ( lastSlash < 0 || lastSlash === blockName.length - 1 ) {
-		return blockName;
-	}
+	const messages = [];
 
-	return blockName.substring( lastSlash + 1 );
+	messages.push( ...getErrorMessages( issues ) );
+	messages.push( ...getWarningMessages( issues ) );
+
+	messages.push( getAnalysisConclusion( validation, issues ) );
+
+	return messages;
 }
 
 /**
@@ -180,7 +164,7 @@ export function sanitizeParentName( parent: string ): string {
  *
  * @returns {string} The presentable warning message, or null if no warnings are found.
  */
-export default function getWarnings( clientId: string ): sidebarWarning[] {
+export default function getWarnings( clientId: string ): SidebarWarning[] {
 	const validation: BlockValidationResult = getValidationResult( clientId );
 	if ( ! validation ) {
 		return null;

packages/schema-blocks/src/functions/validators/innerBlocksValid.ts

@@ -13,6 +13,7 @@ import { getInnerblocksByName } from "../innerBlocksHelper";
 import logger from "../logger";
 import isValidResult from "./isValidResult";
 import { BlockPresence } from "../../core/validation/BlockValidationResult";
+import { getHumanReadableBlockName } from "../BlockHelper";
 
 /**
  * Finds all blocks that should/could be in the inner blocks, but aren't.
@@ -32,7 +33,8 @@ function findMissingBlocks( existingBlocks: BlockInstance[], allBlocks: Required
 
 	// These blocks should've existed, but they don't.
 	return missingBlocks.map( missingBlock =>
-		new BlockValidationResult( null, missingBlock.name, BlockValidation.MissingBlock, blockPresence ) );
+		BlockValidationResult.MissingBlock( getHumanReadableBlockName( missingBlock.name ), blockPresence ),
+	);
 }
 
 /**
@@ -103,7 +105,7 @@ function validateInnerblockTree( blockInstance: BlockInstance ): BlockValidation
  * @returns {BlockValidationResult[]} The names and reasons of the inner blocks that are invalid.
  */
 function validateInnerBlocks( blockInstance: BlockInstance, requiredBlocks: RequiredBlock[] = [],
-							  recommendedBlocks: RecommendedBlock[] = [] ): BlockValidationResult[]  {
+							  recommendedBlocks: RecommendedBlock[] = [] ): BlockValidationResult[] {
 	const requiredBlockKeys = requiredBlocks.map( rblock => rblock.name );
 	const recommendedBlockKeys = recommendedBlocks.map( rblock => rblock.name );
 
@@ -134,7 +136,7 @@ function validateInnerBlocks( blockInstance: BlockInstance, requiredBlocks: Requ
 
 	validationResults = validationResults.filter( result =>
 		! ( isValidResult( result.result ) &&
-		validationResults.some( also => also.clientId === result.clientId && ! isValidResult( also.result ) ) ) );
+			validationResults.some( also => also.clientId === result.clientId && ! isValidResult( also.result ) ) ) );
 
 	return validationResults;
 }