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

P2 786 only render ok when block is valid #1146

Merged
merged 41 commits into from
May 10, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
4f15ce7
Added a method that checks if the blockValidationResult is considered…
Apr 7, 2021
d976c74
Adds a check to see if the added suggestion is valid
Apr 7, 2021
0e3a803
Updates the tests for the blocksuggestions
Apr 7, 2021
f772213
Improval of the test by mocking the functions directly
Apr 7, 2021
bc2a734
Merge branch 'develop' of https://github.com/Yoast/javascript into P2…
Apr 8, 2021
9eb9930
Process CR
Apr 8, 2021
c467972
Remove typehints in jsdoc
Apr 8, 2021
54dfb3a
Merge branch 'develop' of https://github.com/Yoast/javascript into P2…
Apr 8, 2021
cd2ea53
Check if the block validation is OK
Apr 12, 2021
c67c29a
Merge branch 'develop' of https://github.com/Yoast/javascript into P2…
Apr 12, 2021
ebfa7e5
Merge branch 'develop' of https://github.com/Yoast/javascript into P2…
Apr 14, 2021
8e2debd
Refactored the BlockSuggestionsPresenter a little bit
Apr 15, 2021
7404075
Changes the working of the validation a little bit.
Apr 15, 2021
51d120d
make the SidebarWarningPresenter retrieve validation for a client Id
increddibelly Apr 15, 2021
74d36e4
fix unit tests
increddibelly Apr 16, 2021
c89071d
extract the BlockSuggestionsPresenter logic from the component
increddibelly Apr 16, 2021
e95a228
BlockSuggestionsPresenter allows more detailed control now; validatio…
increddibelly Apr 19, 2021
565f3fe
Merge branch 'develop' into P2-786-only-render-ok-when-block-is-valid
increddibelly Apr 19, 2021
0426ce0
added additional valid results and validation
increddibelly Apr 20, 2021
9c10dcf
Add validation to RichtTextBase
increddibelly Apr 20, 2021
e251ab9
remove unused code
increddibelly Apr 20, 2021
25356ce
added ValidationBlockInstruction to allow blocks to opt-in validation
increddibelly Apr 23, 2021
9872f78
fixing unit tests
increddibelly Apr 23, 2021
cd31dfe
fixed unit tests
increddibelly Apr 29, 2021
94a719f
Bump @wordpress/data to the latest version
johannadevos Apr 29, 2021
87a09ca
prevent an error for missing instructions
increddibelly Apr 29, 2021
f9f5aaf
Merge remote-tracking branch 'origin/release/16.3' into P2-786-only-r…
increddibelly Apr 29, 2021
cad6799
replaced ValidatingBlockInstruction base class with single function
increddibelly Apr 30, 2021
9f8404e
Merge remote-tracking branch 'origin/release/16.3' into P2-786-only-r…
increddibelly Apr 30, 2021
1d0e61d
fine tune variationpicker validation
increddibelly May 3, 2021
a41779b
fix Date validation
increddibelly May 3, 2021
3c14f53
typo
increddibelly May 3, 2021
e28dcb0
Merge remote-tracking branch 'origin/develop' into P2-786-only-render…
increddibelly May 3, 2021
8db91db
fix Date tests.
increddibelly May 4, 2021
318b5a3
Update packages/schema-blocks/src/functions/presenters/BlockSuggestio…
increddibelly May 6, 2021
0f4d7b1
Update packages/schema-blocks/src/functions/presenters/BlockSuggestio…
increddibelly May 6, 2021
94fa03a
processed CR comments
increddibelly May 6, 2021
c93634a
Update packages/schema-blocks/src/core/validation/BlockValidationResu…
increddibelly May 6, 2021
3cfa7f1
fix CS
increddibelly May 6, 2021
a92916b
make method of reading current block content consistent with other Yo…
increddibelly May 6, 2021
0053d33
Removed superfluous isValid check from BlockSuggestionsPresenter.
hansjovis May 6, 2021
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
Prev Previous commit
Next Next commit
extract the BlockSuggestionsPresenter logic from the component
  • Loading branch information
increddibelly committed Apr 16, 2021
commit c89071db5ab5b74841578c74e7826e44bf2729ba
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { get } from "lodash";
import { YOAST_SCHEMA_BLOCKS_STORE_NAME } from "../redux";
import { ReactElement } from "react";
import { BlockInstance, createBlock } from "@wordpress/blocks";
import { createBlock } from "@wordpress/blocks";
import { PanelBody } from "@wordpress/components";
import { getValidationResults, getValidationResultForClientId } from "../validators";
import { withSelect } from "@wordpress/data";
import { createElement } from "@wordpress/element";
import { getBlockType } from "../BlockHelper";
import { getInnerblocksByName, insertBlock } from "../innerBlocksHelper";
import { isValidResult } from "../validators/validateResults";
import { insertBlock } from "../innerBlocksHelper";
import { isMissingResult, isValidResult } from "../validators/validateResults";
import { BlockValidationResult } from "../../core/validation";

type BlockSuggestionAddedDto = {
suggestedBlockTitle: string;
Expand All @@ -20,10 +22,20 @@ type BlockSuggestionDto = {

interface BlockSuggestionsProps {
heading: string;
parentBlock: BlockInstance;
suggestedBlockNames: string[];
parentClientId: string;
blockNames: string[];
}

export type SuggestionDetails = BlockValidationResult & {
title: string;
}

type SuggestionDto = {
heading: string;
parentClientId: string;
suggestions: SuggestionDetails[];
};

/**
* Renders a block suggestion with the possibility to add one.
*
Expand All @@ -48,6 +60,7 @@ function BlockSuggestion( { suggestedBlockTitle, suggestedBlockName, parentBlock
</li>
);
}

/**
* Renders a block suggestion that has already been added.
*
Expand All @@ -71,58 +84,64 @@ function BlockSuggestionAdded( { suggestedBlockTitle, isValid }: BlockSuggestion
}

/**
* Renders a list of suggested blocks.
*
* @param props The props.
*
* @returns The block suggestions element.
* Renders a list of block suggestions.
* @param props The BlockValidationResults and the Blocks' titles.
* @returns The appropriate Block Suggestion elements.
*/
export default function BlockSuggestionsPresenter( { heading, parentBlock, suggestedBlockNames }: BlockSuggestionsProps ) {
// Filters all suggested blocks that doesn't have a blockType registered.
const filteredSuggestedBlockNames = suggestedBlockNames
.filter( suggestedBlock => typeof getBlockType( suggestedBlock ) !== "undefined" );

// When there are no suggestions, just return.
if ( filteredSuggestedBlockNames.length === 0 ) {
return null;
}

const presentBlocks = getInnerblocksByName( parentBlock, filteredSuggestedBlockNames );
const validationResults = getValidationResults();

export function PureBlockSuggestionsPresenter( { heading, parentClientId, suggestions }: SuggestionDto ): ReactElement {
return (
<PanelBody key={ heading + parentBlock.clientId }>
<PanelBody key={ heading + parentClientId }>
<div className="yoast-block-sidebar-title">{ heading }</div>
<ul className="yoast-block-suggestions">
{
filteredSuggestedBlockNames.map( ( suggestedBlockName: string, index: number ) => {
const suggestedBlockType = getBlockType( suggestedBlockName );
suggestions.map( ( suggestion, index: number ) => {
// If the validation was found, and it is completely OK, we will add a check mark.
const isValid = suggestion && isValidResult( suggestion.result );
const isMissing = suggestion && isMissingResult( suggestion.result );

const existingBlock = presentBlocks.find( block => block.name === suggestedBlockName );
if ( existingBlock ) {
// Find the validation result for this block.
const validation = getValidationResultForClientId( existingBlock.clientId, validationResults );
// If the validation was found, and it is completely OK, we will add a check mark.
const isTheBlockValid = validation && isValidResult( validation.result );

// Show the validation result.
return <BlockSuggestionAdded
if ( isMissing ) {
// Show the suggestion to add an instance of this block.
return <BlockSuggestion
key={ index }
isValid={ isTheBlockValid }
suggestedBlockTitle={ suggestedBlockType.title }
suggestedBlockTitle={ suggestion.title }
suggestedBlockName={ suggestion.name }
parentBlockClientId={ parentClientId }
/>;
}

// Show the suggestion to add an instance of this block.
return <BlockSuggestion
// Show the validation result.
return <BlockSuggestionAdded
key={ index }
suggestedBlockTitle={ suggestedBlockType.title }
suggestedBlockName={ suggestedBlockName }
parentBlockClientId={ parentBlock.clientId }
isValid={ isValid }
suggestedBlockTitle={ suggestion.title }
/>;
} )
}
</ul>
</PanelBody>
);
}

/**
* Renders a list of suggested blocks.
*
* @param props The props.
*
* @returns The block suggestions element.
*/
export default withSelect<Partial<SuggestionDto>, BlockSuggestionsProps, SuggestionDto>( ( select, props: BlockSuggestionsProps ) => {
const validations: BlockValidationResult[] =
select( YOAST_SCHEMA_BLOCKS_STORE_NAME ).getInnerblockValidations( props.parentClientId, props.blockNames );

const suggestionDetails = validations.map( validation => {
const type = select( "core/blocks" ).getBlockType( validation.name );
return {
title: get( type, "title", "" ),
...validation,
};
} );

return {
suggestions: suggestionDetails,
};
} )( PureBlockSuggestionsPresenter );
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ function useBlock( clientId: string ): BlockInstance {
* @constructor
*/
export function InnerBlocksSidebar( props: InnerBlocksSidebarProps ): ReactElement {
const block = useBlock( props.currentBlock.clientId );
const validationResults = useValidationResults( props.currentBlock.clientId );

let warnings: SidebarWarning[] = [];
Expand All @@ -70,13 +69,13 @@ export function InnerBlocksSidebar( props: InnerBlocksSidebarProps ): ReactEleme
<WarningList warnings={ warnings } />
<BlockSuggestions
heading={ __( "Required Blocks", "yoast-schema-blocks" ) }
parentBlock={ block }
suggestedBlockNames={ props.requiredBlocks }
parentClientId={ props.currentBlock.clientId }
blockNames={ props.requiredBlocks }
/>
<BlockSuggestions
heading={ __( "Recommended Blocks", "yoast-schema-blocks" ) }
parentBlock={ block }
suggestedBlockNames={ props.recommendedBlocks }
parentClientId={ props.currentBlock.clientId }
blockNames={ props.recommendedBlocks }
/>
</Fragment>;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,60 @@ export type ClientIdValidation = Record<string, BlockValidationResult>;
*
* @returns {Record<string, BlockValidationResult>} The schema blocks validation results.
*/
export function getSchemaBlocksValidationResults( state: SchemaBlocksState ): ClientIdValidation {
export function getValidationResults( state: SchemaBlocksState ): ClientIdValidation {
return state.validations || SchemaBlocksDefaultState.validations;
}


/**
* Recursively traverses a BlockValidationResult's issues to finds the validation results for a specific clientId.
* @param state The entire Schema Blocks state.
* @param clientId The ClientId of the block you want validation results for.
* @returns The BlockValidationResult matching the clientId or null if none were found.
*/
export function getValidationResultForClientId( state: SchemaBlocksState, clientId: string ): BlockValidationResult {
const stored = getValidationResults( state );
const validationResults = Object.values( stored );

return recursivelyFind( validationResults, ( result ) => result.clientId === clientId );
}

/**
* Finds all validation results for innerblocks that match the names of given blocks.
* @param state The entire Schema Blocks state.
* @param clientId The clientId of the parent block containing the Innerblocks, e.g. the job posting id.
* @param blockNames the set of blocknames you're looking for.
* @returns The innerblock's validation.
*/
export function getInnerblockValidations( state: SchemaBlocksState, clientId: string, blockNames?: string[] ) {
const validation = getValidationResultForClientId( state, clientId );
if ( ! validation || validation.issues.length <= 0 ) {
return null;
}
return recursivelyFind( validation.issues, ( result ) => blockNames.includes( result.name ) );
}

/**
* Recursively traverses a BlockValidationResult's issues to finds the validation results for a specific clientId.
* @param source The validation results to process.
* @param predicate The predicate that determines wether to filter the validation or not.
* @returns The BlockValidationResult matching the clientId or null if none were found.
*/
function recursivelyFind( source: BlockValidationResult[], predicate: ( source: BlockValidationResult ) => boolean ): BlockValidationResult {
for ( const validationResult of source ) {
// When the validation result matches the client id, return it.
if ( predicate( validationResult ) ) {
return validationResult;
}

// Just keep driving down the tree calling until we have found the result.
if ( validationResult.issues.length > 0 ) {
const validation = recursivelyFind( validationResult.issues, predicate );
if ( validation ) {
return validation;
}
}
}
// We haven't found the result down this tree.
return null;
}
59 changes: 38 additions & 21 deletions packages/schema-blocks/src/functions/validators/validateResults.ts
Original file line number Diff line number Diff line change
@@ -1,47 +1,64 @@
import { BlockValidation } from "../../core/validation";

/**
* Determines if a specific validation result is valid.
* Determines if a specific validation source is valid.
*
* @param result The source value.
* @param source The source value.
*
* @returns Whether the result is valid.
* @returns Whether the source is valid.
*/
function isValidResult( result: BlockValidation ): boolean {
return result < 200;
export function isValidResult( source: BlockValidation ): boolean {
return source < 200;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return source < 200;
return source < BlockValidation.OK;

I think it will be better to not use a magic number, but to explicitly reference it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also for the other 'status codes' in this file.

}

/**
* Determines if the result should lead to Schema output.
* Determines if a specific validation indicates if something is missing.
*
* @param result The source value.
* @param source The validation to check.
*
* @returns Whether the result should lead to Schema output.
* @returns Wether the source indicates something is missing.
*/
function isResultValidForSchema( result: BlockValidation ): boolean {
return result < 300;
export function isMissingResult( source: BlockValidation ): boolean {
return [
BlockValidation.MissingRecommendedAttribute,
BlockValidation.MissingRecommendedBlock,
BlockValidation.MissingRecommendedVariation,

BlockValidation.MissingRequiredAttribute,
BlockValidation.MissingRequiredBlock,
BlockValidation.MissingRequiredVariation,
].includes( source );
}

/**
* Determines if the result is OK (in other words, would lead to an orange bullet).
* Determines if the source should lead to Schema output.
*
* @param result The source value.
* @param source The source value.
*
* @returns Whether the result is OK.
* @returns Whether the source should lead to Schema output.
*/
function isOkResult( result: BlockValidation ): boolean {
return result >= 200 && result < 300;
export function isResultValidForSchema( source: BlockValidation ): boolean {
return source < 300;
}

/**
* Determines if the result is invalid.
* Determines if the source is OK (in other words, would lead to an orange bullet).
*
* @param result The source value.
* @param source The source value.
*
* @returns Whether the result is invalid.
* @returns Whether the source is OK.
*/
function isInvalidResult( result: BlockValidation ): boolean {
return result >= 300;
export function isOkResult( source: BlockValidation ): boolean {
return source >= 200 && source < 300;
}

export { isValidResult, isResultValidForSchema, isOkResult, isInvalidResult };
/**
* Determines if the source is invalid.
*
* @param source The source value.
*
* @returns Whether the source is invalid.
*/
export function isInvalidResult( source: BlockValidation ): boolean {
return source >= 300;
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ jest.mock( "@wordpress/data", () => ( {
dispatch: jest.fn( () => ( {
insertBlock: jest.fn(),
} ) ),
withSelect: jest.fn( () => jest.fn() ),
} ) );

jest.mock( "@wordpress/components", () => {
Expand Down
Loading