Skip to content
This repository has been archived by the owner on Oct 4, 2022. It is now read-only.

Added title block instruction. #1129

Merged
merged 6 commits into from
Mar 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { BlockValidation } from ".";
import { BlockInstance } from "@wordpress/blocks";
import { __, sprintf } from "@wordpress/i18n";

export enum BlockPresence {
Required = "required",
Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -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.
*
Expand Down
23 changes: 22 additions & 1 deletion packages/schema-blocks/src/functions/BlockHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -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";
}

/**
Expand All @@ -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" };
}

/**
Expand All @@ -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;
}

/**
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 ),
);
}

/**
Expand Down Expand Up @@ -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 );

Expand Down Expand Up @@ -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;
}
Expand Down
Loading