Show an orange bullet in the sidebar analysis when a recommended block is removed by johannadevos · Pull Request #1116 · Yoast/javascript

johannadevos · 2021-03-23T14:57:12Z

Summary

This PR can be summarized in the following changelog entry:

[schema-blocks] Adds an orange bullet to the Analysis in the side bar when a recommended block is removed.

Relevant technical choices:

I added an enum Blocktype in BlockValidationResult.ts, which holds the different block types (required, recommended, optional, and for the time being, also unknown).
I added a blockType parameter to the BlockValidationResult class, with the type being BlockType.
As a result, everywhere where the BlockVariationResult class is instantiated, it needs to be passed a blockType parameter.
This is why I also added unknown as a value to the enum; I'm passing unknown to all instantiations of BlockValidationResult which are outside the scope of this PR. We could look into fixing this, but I'm not sure it worth the time investment right now. The thing is that we don't know the block type for many block instances, since the block templates themselves don't pass their own block type (instead, it is listed in job-posting.block.php what is the block type for each block).
The validateInnerBlocks function in innerBlocksValid.ts now also calls the findMissingBlocks function for recommended blocks (it used to call this function only for required blocks).
I did some renames in the findMissingBlocks function to reflect this.
For each missing/removed block, the findMissingBlocks function adds the block type to the BlockValidationResult. These results will be used in the sidebar (as the status of the messageData in SidebarWarningPresenter.ts) to show the appropriate warning.

Test instructions

This PR can be tested by following these steps:

Add the Yoast Job Posting block.
See that there are no orange bullets in the Analysis in the side bar, because all the recommended blocks are present upon initialization.
You should see a red bullet saying "Not all required blocks are complete! No job posting schema will be generated for your page." (this is because title, description and location have not yet been filled in).

Test scenario 7

Remove a recommended block, for example the 'Requirements'.
See an orange bullet and the text: "The 'Requirements' block is recommended but missing.":
See that the Requirements block is greyed out and can be added:
Remove another recommended block and see another orange bullet appear (with the right text).
The red bullet saying "Not all required blocks are complete! No job posting schema will be generated for your page." should still be present.

Test scenario 8

Fill in the title, description and location.
The red bullet should become green and say: "Good job! All required blocks are complete.".
You should still see one or more orange bullets corresponding to the recommended blocks you have removed.
Remove a required block and see that there are now two red bullets: one saying which block is missing, and one saying that not all required blocks are completed.

Test Schema output

Fill in a recommended block and see that it is present in the Schema output.

Note that scenarios 7 and 8 both contain a line regarding the color of the emoticon in the pre-publish check; I didn't solve that yet as there is a separate ticket for this task: https://yoast.atlassian.net/browse/P2-789.

Impact check

This PR affects the following parts of the plugin, which may require extra testing:
*

UI changes

This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Quality assurance

I have tested this code to the best of my abilities
I have added unittests to verify the code works as intended

Fixes https://yoast.atlassian.net/browse/P2-706

…Blocks.tsx

…ng, always set its value to Unknown

…ive to recommended blocks

It is no longer equal to the RequiredBlockType

…issingBlocks function

So that they appear as lower case in the analysis message in the sidebar

…nded rather than required

…commended blocks with issues

…ting tests Also improve some CS

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

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

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

increddibelly

zie opmerkingen; het zou nog zeer waardevol zijn om dit deel over te dragen aan de onderliggende functie dwz dat daar de splitsing op recommended / required wordt gemaakt, maar alleen als het in een uur of twee kost.

…-706-orange-analysis

It's not the issues who are required, but the blocks

…r a need to pass this value through the `job-posting.block.template`

…as well

andizer

CR done

andizer · 2021-03-29T12:05:46Z

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

 	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" ) );


Hmm, this will cause merge conflicts with #1127 where this logic (replaceVariables) has been removed.

Also please not that dynamic arguments for translations aren't translatable. The potfile builder will take issue.presence as value to translate.

Good to know!

andizer · 2021-03-29T12:06:33Z

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

 		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 +


This logic has also been changed in #1127

andizer · 2021-03-29T12:08:38Z

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

+
 	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;


Tip: I would make this more readable by adding newlines. Especially the conditional makes this all hard to read/process

Good point. But I believe this code has been changed in #1127 anyway.

johannadevos · 2021-03-29T12:51:18Z

I'm merging this PR with the blessing of the other devs in the team.

It has already been acceptance tested in a demo with @dariaknl.

The comments from @andizer's CR all relate to conflicts with #1127. We will resolve those conflicts after this PR has been merged to develop.

johannadevos added 13 commits March 19, 2021 15:09

Minor CS fixes

6d4b06e

WIP: Add missing recommended blocks to the validation.issues in Inner…

abf4526

…Blocks.tsx

Add the BlockType enum to BlockValidationResult and, for the time bei…

3aea2f5

…ng, always set its value to Unknown

Rename the variables in findMissingBlocks to make the function inclus…

99477c2

…ive to recommended blocks

Pass the block type to the findMissingBlocks() function

7031766

Convert the numeric enum to a string enum

8cfa903

Improve CS in imports

84014f8

Improve naming in findMissingBlocks function

96af683

Undo changes made to the RecommendedBlock type

b2edab2

It is no longer equal to the RequiredBlockType

Fix TS error where recommended blocks couldn't be passed to the findM…

47ceefc

…issingBlocks function

Improve function documentation

40a0c40

Convert the enum values to lower case

fa7fa29

So that they appear as lower case in the analysis message in the sidebar

Show an orange bullet in the analysis if the missing block is recomme…

abc47c0

…nded rather than required

johannadevos added innovation changelog: enhancement Needs to be included in the 'Enhancements' category in the changelog labels Mar 23, 2021

johannadevos added this to the 16.2 milestone Mar 23, 2021

Make sure the analysis conclusion is still shown in case there are re…

c562969

…commended blocks with issues

johannadevos added the UI change PRs that result in a change in the UI label Mar 23, 2021

johannadevos added 8 commits March 23, 2021 17:29

Improve copy for the sidebar analysis

b8a4546

Fix CS in imports

bc81d23

Fix more CS in imports

2a36ceb

Improve documentation

a99a09d

Fix eslint errors about the maximum length of a line

c3572ed

Add the new BlockType parameter for the BlockValidationResult to exis…

53fa3a3

…ting tests Also improve some CS

Add tests for recommended blocks

248cd24

Test the scenario where the validation results cannot be retrieved

4f4e252