Merge pull request #1116 from Yoast/P2-706-orange-analysis

Show an orange bullet in the sidebar analysis when a recommended block is removed

Author: johannadevos

packages/schema-blocks/src/core/Definition.ts

@@ -4,6 +4,7 @@ import { isArray, mergeWith } from "lodash";
 import Instruction from "./Instruction";
 import Leaf from "./Leaf";
 import logger from "../functions/logger";
+import { BlockPresence } from "./validation/BlockValidationResult";
 
 export type DefinitionClass<T extends Definition> = {
 	new( separator: string, template?: string, instructions?: Record<string, Instruction>, tree?: Leaf ): T;
@@ -80,7 +81,7 @@ export default abstract class Definition {
 			return null;
 		}
 
-		const validation = new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Unknown );
+		const validation = new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Unknown, BlockPresence.Unknown );
 
 		logger.startGroup( `Validation results: ${ blockInstance.name }` );
 

packages/schema-blocks/src/core/Instruction.ts

@@ -1,6 +1,7 @@
 import { BlockInstance } from "@wordpress/blocks";
 import logger from "../functions/logger";
 import { BlockValidationResult, BlockValidation } from "./validation";
+import { BlockPresence } from "./validation/BlockValidationResult";
 export type InstructionPrimitive = string | number | boolean;
 export type InstructionValue = InstructionPrimitive | InstructionObject | InstructionArray;
 export type InstructionArray = readonly InstructionValue[];
@@ -60,7 +61,7 @@ export default abstract class Instruction {
 	 * @returns {BlockValidationResult} The validation result.
 	 */
 	validate( blockInstance: BlockInstance ): BlockValidationResult {
-		return new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Unknown );
+		return new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Unknown, BlockPresence.Unknown );
 	}
 
 	/**

packages/schema-blocks/src/core/blocks/BlockInstruction.ts

@@ -1,12 +1,13 @@
 import BlockLeaf from "./BlockLeaf";
-import { RenderSaveProps, RenderEditProps } from "./BlockDefinition";
+import { RenderEditProps, RenderSaveProps } from "./BlockDefinition";
 import { ReactElement } from "@wordpress/element";
 import { BlockConfiguration, BlockInstance } from "@wordpress/blocks";
-import { BlockValidationResult, BlockValidation } from "../validation";
+import { BlockValidation, BlockValidationResult } from "../validation";
 import Instruction, { InstructionOptions } from "../Instruction";
 import { attributeExists, attributeNotEmpty } from "../../functions/validators";
 import validateMany from "../../functions/validators/validateMany";
 import logger from "../../functions/logger";
+import { BlockPresence } from "../validation/BlockValidationResult";
 
 export type BlockInstructionClass = {
 	new( id: number, options: InstructionOptions ): BlockInstruction;
@@ -78,16 +79,18 @@ export default abstract class BlockInstruction extends Instruction {
 	 * @returns {BlockValidationResult} The validation result.
 	 */
 	validate( blockInstance: BlockInstance ): BlockValidationResult {
-		const validation = new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Skipped );
+		const validation = new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Skipped, BlockPresence.Unknown );
 
 		if ( this.options && this.options.required === true ) {
 			const attributeValid = attributeExists( blockInstance, this.options.name as string ) &&
 						           attributeNotEmpty( blockInstance, this.options.name as string );
 			if ( attributeValid ) {
-				validation.issues.push( new BlockValidationResult( blockInstance.clientId, this.options.name, BlockValidation.Valid ) );
+				validation.issues.push( new BlockValidationResult( blockInstance.clientId, this.options.name,
+					BlockValidation.Valid, BlockPresence.Unknown ) );
 			} else {
 				logger.warning( "block " + blockInstance.name + " has a required attributes " + this.options.name + " but it is missing or empty" );
-				validation.issues.push( new BlockValidationResult( blockInstance.clientId, this.options.name, BlockValidation.MissingAttribute ) );
+				validation.issues.push( new BlockValidationResult( blockInstance.clientId, this.options.name,
+					BlockValidation.MissingAttribute, BlockPresence.Unknown ) );
 				validation.result = BlockValidation.Invalid;
 			}
 		} else {

packages/schema-blocks/src/core/schema/SchemaInstruction.ts

@@ -1,7 +1,8 @@
-import { SchemaValue, SchemaDefinitionConfiguration } from "./SchemaDefinition";
+import { SchemaDefinitionConfiguration, SchemaValue } from "./SchemaDefinition";
 import Instruction, { InstructionOptions } from "../Instruction";
 import { BlockInstance } from "@wordpress/blocks";
 import { BlockValidation, BlockValidationResult } from "../validation";
+import { BlockPresence } from "../validation/BlockValidationResult";
 
 export type SchemaInstructionClass = { new( id: number, options: InstructionOptions ): SchemaInstruction };
 
@@ -39,6 +40,6 @@ export default abstract class SchemaInstruction extends Instruction {
 	 * @returns {BlockValidationResult} The validation results.
 	 */
 	validate( blockInstance: BlockInstance ): BlockValidationResult {
-		return new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Valid );
+		return new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Valid, BlockPresence.Unknown );
 	}
 }

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

@@ -1,4 +1,3 @@
-
 export enum BlockValidation {
 	/** This block was skipped during validation, on purpose. If you ever see this value, that's a bug.  */
 	Skipped = -2,

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

@@ -1,6 +1,13 @@
 import { BlockValidation } from ".";
 import { BlockInstance } from "@wordpress/blocks";
 
+export enum BlockPresence {
+	Required = "required",
+	Recommended = "recommended",
+	Optional = "optional",
+	Unknown = "unknown"
+}
+
 /**
  * Contains the result of a block validation.
  */
@@ -16,24 +23,31 @@ export class BlockValidationResult {
 	public clientId: string;
 
 	/**
-	 * The validation result;
+	 * The validation result.
 	 */
 	public result: BlockValidation;
 
+	/**
+	 * The block presence.
+	 */
+	public blockPresence: BlockPresence;
+
 	/**
 	 * The validation issues for this block's innerblocks or attributes, if any.
 	 */
 	public issues: BlockValidationResult[]
 
 	/**
-	 * @param clientId The clientId of the validated block.
-	 * @param name     The name of the validated block.
-	 * @param result   The validation result.
+	 * @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.
 	 */
-	constructor( clientId: string, name: string, result: BlockValidation ) {
-		this.name = name;
+	constructor( clientId: string, name: string, result: BlockValidation, blockPresence: BlockPresence ) {
 		this.clientId = clientId;
+		this.name = name;
 		this.result = result;
+		this.blockPresence = blockPresence;
 		this.issues = [];
 	}
 
@@ -50,6 +64,7 @@ export class BlockValidationResult {
 			blockInstance.clientId,
 			name || blockInstance.name,
 			BlockValidation.MissingAttribute,
+			BlockPresence.Unknown,
 		);
 	}
 
@@ -66,6 +81,7 @@ export class BlockValidationResult {
 			blockInstance.clientId,
 			name || blockInstance.name,
 			BlockValidation.Valid,
+			BlockPresence.Unknown,
 		);
 	}
 }

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

@@ -1,5 +1,5 @@
-import { RequiredBlockOption } from ".";
 import { InstructionOptions } from "../Instruction";
+import { RequiredBlockOption } from ".";
 import { SuggestedBlockProperties } from "./SuggestedBlockProperties";
 
 /**

packages/schema-blocks/src/functions/gutenberg/watch.ts

@@ -7,6 +7,7 @@ import { getBlockDefinition } from "../../core/blocks/BlockDefinitionRepository"
 import { BlockValidation, BlockValidationResult } from "../../core/validation";
 import storeBlockValidation from "./storeBlockValidation";
 import logger from "../logger";
+import { BlockPresence } from "../../core/validation/BlockValidationResult";
 
 let updatingSchema = false;
 let previousRootBlocks: BlockInstance[];
@@ -104,7 +105,7 @@ export function validateBlocks( blocks: BlockInstance[] ): BlockValidationResult
 			validations.push( definition.validate( block ) );
 		} else {
 			logger.warning( "Unable to validate block of type [" + block.name + "] " + block.clientId );
-			validations.push( new BlockValidationResult( block.clientId, block.name, BlockValidation.Unknown ) );
+			validations.push( new BlockValidationResult( block.clientId, block.name, BlockValidation.Unknown, BlockPresence.Unknown ) );
 
 			// Recursively validate all blocks' innerblocks.
 			if ( block.innerBlocks && block.innerBlocks.length > 0 ) {

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

@@ -11,7 +11,7 @@ import { SvgIcon } from "@yoast/components";
  * Renders warnings and appenders for any block's InnerBlocks.
  *
  * @param currentBlock The innerblocks instance.
- * @param options The InnerBlocksInstructionOptions of the innerblock.
+ * @param options      The InnerBlocksInstructionOptions of the innerblock.
  *
  * @returns {ReactElement[]} React Elements representing the sidebar elements for these innerblocks.
  */

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

@@ -4,9 +4,10 @@ import { __ } from "@wordpress/i18n";
 import { get } from "lodash";
 import { BlockValidationResult } from "../../core/validation";
 import { BlockValidation } from "../../core/validation";
+import { BlockPresence } from "../../core/validation/BlockValidationResult";
 
 const analysisMessageTemplates: Record<number, string> = {
-	[ BlockValidation.MissingBlock ]: "The '{child}' block is {status} but missing.",
+	[ BlockValidation.MissingBlock ]: "The '{child}' block is {presence} but missing.",
 	[ BlockValidation.MissingAttribute ]: "The '{child}' block is empty.",
 };
 
@@ -16,7 +17,7 @@ type analysisIssue = {
 	name: string;
 	parent: string;
 	result: BlockValidation;
-	status: string;
+	presence: string;
 };
 
 export type sidebarWarning = {
@@ -51,7 +52,7 @@ 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( "{status}", __( issue.status, "wpseo-schema-blocks" ) );
+		.replace( "{presence}", __( issue.presence, "wpseo-schema-blocks" ) );
 }
 
 /**
@@ -63,20 +64,26 @@ export function replaceVariables( issue: analysisIssue ): string {
  * @returns {sidebarWarning} Any analysis conclusions that should be in the footer.
  */
 function getAnalysisConclusion( validation: BlockValidation, issues: analysisIssue[] ): sidebarWarning {
-	if ( issues.some( issue => issue.result === BlockValidation.MissingBlock ||
+	// 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 are completed! No " + issues[ 0 ].parent +
+			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;
 	}
 
+	// Show a green bullet when all required blocks have been completed.
+	const requiredBlockIssues = issues.filter( issue => {
+		return issue.presence === BlockPresence.Required;
+	} );
+
 	if ( validation === BlockValidation.Valid ||
-		issues.every( issue => issue.result !== BlockValidation.MissingAttribute &&
+		requiredBlockIssues.every( issue => issue.result !== BlockValidation.MissingAttribute &&
 			issue.result !== BlockValidation.MissingBlock ) ) {
 		return {
-			text: __( "Good job! All required blocks are completed.", "wpseo-schema-blocks" ),
+			text: __( "Good job! All required blocks have been completed.", "wpseo-schema-blocks" ),
 			color: "green",
 		} as sidebarWarning;
 	}
@@ -87,7 +94,7 @@ function getAnalysisConclusion( validation: BlockValidation, issues: analysisIss
  *
  * @param validation The root validation result.
  *
- * @return all validation results.
+ * @return All validation results.
  */
 function getAllDescendantIssues( validation: BlockValidationResult ): BlockValidationResult[] {
 	let results = [ validation ];
@@ -114,10 +121,11 @@ export function createAnalysisMessages( validation: BlockValidationResult ): sid
 			name: sanitizeBlockName( issue.name ),
 			parent: sanitizeParentName( parent ),
 			result: issue.result,
-			status: "required",
+			presence: issue.blockPresence,
 		} ) );
+
 	const messages = messageData.map( msg => {
-		return { text: replaceVariables( msg ), color: "red" } as sidebarWarning;
+		return { text: replaceVariables( msg ), color: ( msg.presence === BlockPresence.Recommended ? "orange" : "red" ) } as sidebarWarning;
 	} );
 
 	const conclusion = getAnalysisConclusion( validation.result, messageData );