P2-704 Same Post and Job titles do not trigger red analysis#1120
P2-704 Same Post and Job titles do not trigger red analysis#1120
Conversation
…ascript into P2-704-same-post-and-job-title
hansjovis
added
the
changelog: non-user-facing
increddibelly
left a comment
increddibelly
left a comment
There was a problem hiding this comment.
only a few comments for minor improvements.
also, /src/functions/blocks/index.ts is an empty file, please remove
| import { createElement } from "@wordpress/element"; | ||
| import { BlockInstance } from "@wordpress/blocks"; | ||
| import BlockSuggestions from "../../blocks/BlockSuggestions"; | ||
| import { RequiredBlocks as BlockSuggestions } from "../../blocks/BlockSuggestions"; |
| import attributeExists from "./attributeExists"; | ||
| import attributeNotEmpty from "./attributeNotEmpty"; | ||
| import getInvalidInnerBlocks from "./innerBlocksValid"; | ||
| import { validateInnerBlocks as getInvalidInnerBlocks } from "./innerBlocksValid"; |
There was a problem hiding this comment.
why the rename?
| import { validateInnerBlocks as getInvalidInnerBlocks } from "./innerBlocksValid"; | |
| import { validateInnerBlocks } from "./innerBlocksValid"; |
| @@ -6,6 +6,7 @@ import recurseOverBlocks from "../blocks/recurseOverBlocks"; | |||
| import { getInnerblocksByName } from "../innerBlocksHelper"; | |||
| import logger from "../logger"; | |||
| import isValidResult from "./isValidResult"; | |||
There was a problem hiding this comment.
there's still a default export remaining here.
| @@ -3,10 +3,10 @@ import { BlockEditProps, BlockConfiguration } from "@wordpress/blocks"; | |||
| import { createElement } from "@wordpress/element"; | |||
| import { SelectControl } from "@wordpress/components"; | |||
| // Internal imports. | |||
There was a problem hiding this comment.
| // Internal imports. |
|
|
||
| import VariableTagRichText from "./VariableTagRichText"; | ||
|
|
||
| export { VariableTagRichText }; |
There was a problem hiding this comment.
why import the rest if we're not exporting them?
There was a problem hiding this comment.
For the BlockInstruction.register side effects.
Without them the block instructions are not registered.
There was a problem hiding this comment.
But yes, I will export them all.
| import "./Schema"; | ||
| import SchemaInstruction from "./Schema"; | ||
|
|
||
| // Need to export something for the side-effects of the imports to work... |
There was a problem hiding this comment.
dank voor dit comment!
kunnen we de side effects expliciet maken / aanroepen?
| import { BlockInstance } from "@wordpress/blocks"; | ||
|
|
||
| import { removeBlock, restoreBlock, getBlockType } from "../../src/functions/BlockHelper"; | ||
| import { removeBlock, restoreBlock, getBlockType, getHumanReadableBlockName } from "../../src/functions/BlockHelper"; |
There was a problem hiding this comment.
rename the file from BlocksHelper.test.ts to BlockHelper.test.ts (singular)
| @@ -1,8 +1,7 @@ | |||
| import { BlockValidation, BlockValidationResult } from "../../../src/core/validation"; | |||
| import getWarnings, { createAnalysisMessages, sanitizeBlockName } from "../../../src/functions/presenters/SidebarWarningPresenter"; | |||
| import getWarnings, { createAnalysisMessages } from "../../../src/functions/presenters/SidebarWarningPresenter"; | |||
There was a problem hiding this comment.
can we un-default export the getWarnings?
|
Closed in favour of #1129. |
2 participants
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
BlockValidationResults itself, rather than a separate result -> warning map.Test instructions
This PR can be tested by following these steps:
Impact check
UI changes
Quality assurance
Fixes #