From ec509c3a9588a100eebd6b4653328ee1d3e6a282 Mon Sep 17 00:00:00 2001 From: Jackie6 <541172791@qq.com> Date: Mon, 8 Apr 2019 17:41:17 -0400 Subject: [PATCH 1/6] Add headings hierarchy checker notice --- packages/block-library/src/heading/edit.js | 3 + .../src/heading/heading-checker.js | 93 +++++++++++++++++++ .../src/heading/style.native.scss | 4 + 3 files changed, 100 insertions(+) create mode 100644 packages/block-library/src/heading/heading-checker.js diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index cf723896a55a03..c92bb339b7b8f8 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -2,6 +2,7 @@ * Internal dependencies */ import HeadingToolbar from './heading-toolbar'; +import HeadingChecker from './heading-checker'; /** * WordPress dependencies @@ -24,6 +25,7 @@ export default function HeadingEdit( { insertBlocksAfter, onReplace, className, + clientId, } ) { const { align, content, level, placeholder } = attributes; const tagName = 'h' + level; @@ -37,6 +39,7 @@ export default function HeadingEdit( {

{ __( 'Level' ) }

setAttributes( { level: newLevel } ) } /> +

{ __( 'Text Alignment' ) }

{ + return flatMap( blocks, ( block = {} ) => { + if ( block.name === 'core/heading' ) { + return { + ...block, + path, + headingId: block.clientId, + level: block.attributes.level, + isEmpty: isEmptyHeading( block ), + }; + } + return computeOutlineHeadings( block.innerBlocks, [ ...path, block ] ); + } ); +}; + +const isEmptyHeading = ( heading ) => ! heading.attributes.content || heading.attributes.content.length === 0; + +export const HeadingChecker = ( { blocks = [], selectedLevel, selectedHeadingId } ) => { + const headings = computeOutlineHeadings( blocks ); + + // Iterate headings to find prevHeadingLevel + let prevHeadingLevel = 1; + let i = 0; + for ( i = 0; i < headings.length; i++ ) { + if ( i >= 1 && headings[ i ].headingId === selectedHeadingId ) { + prevHeadingLevel = headings[ i - 1 ].level; + } + } + + const isIncorrectLevel = ( selectedLevel === 1 || selectedLevel > prevHeadingLevel + 1 ); + + if ( ! isIncorrectLevel ) { + return null; + } + + // The correct heading level should be within [H2, prevHeadingLevel+1] + let suggestedHeading = 'H2'; + for ( i = 3; i < prevHeadingLevel + 2; i++ ) { + suggestedHeading = suggestedHeading + ', H' + i; + } + + const msg = __( 'This heading level is incorrect. Please use ' + suggestedHeading + ' instead.' ); + + // For accessibility + useEffect( () => { + speak( __( 'This heading level is incorrect' ) ); + }, [ selectedLevel ] ); + + return ( +
+ + { msg } + +
+ ); +}; + +export default compose( + withSelect( ( select ) => { + const { getBlocks } = select( 'core/block-editor' ); + + return { + blocks: getBlocks(), + }; + } ) +)( HeadingChecker ); diff --git a/packages/block-library/src/heading/style.native.scss b/packages/block-library/src/heading/style.native.scss index f4801b92a6bc50..e1be684c71ee93 100644 --- a/packages/block-library/src/heading/style.native.scss +++ b/packages/block-library/src/heading/style.native.scss @@ -2,3 +2,7 @@ .blockText { min-height: $min-height-heading; } +.editor-heading-checker +.block-editor-heading-checker { + margin: 0; +} From 3d28ae59a3eb9b7fba303df4e8cd0944227efbd8 Mon Sep 17 00:00:00 2001 From: Jackie6 <541172791@qq.com> Date: Tue, 9 Apr 2019 18:59:07 -0400 Subject: [PATCH 2/6] Remove duplicate function and use import instead --- .../src/heading/heading-checker.js | 45 +++---------------- .../src/components/document-outline/index.js | 2 +- 2 files changed, 6 insertions(+), 41 deletions(-) diff --git a/packages/block-library/src/heading/heading-checker.js b/packages/block-library/src/heading/heading-checker.js index b1eec2bc95ef2e..3bf3e9fa257349 100644 --- a/packages/block-library/src/heading/heading-checker.js +++ b/packages/block-library/src/heading/heading-checker.js @@ -1,8 +1,3 @@ -/** - * External dependencies - */ -import { flatMap } from 'lodash'; - /** * WordPress dependencies */ @@ -14,33 +9,9 @@ import { compose } from '@wordpress/compose'; import { withSelect } from '@wordpress/data'; /** -* Returns an array of heading blocks enhanced with the following properties: -* path - An array of blocks that are ancestors of the heading starting from a top-level node. -* Can be an empty array if the heading is a top-level node (is not nested inside another block). -* level - An integer with the heading level. -* isEmpty - Flag indicating if the heading has no content. -* -* @param {?Array} blocks An array of blocks. -* @param {?Array} path An array of blocks that are ancestors of the blocks passed as blocks. -* -* @return {Array} An array of heading blocks enhanced with the properties described above. -*/ -const computeOutlineHeadings = ( blocks = [], path = [] ) => { - return flatMap( blocks, ( block = {} ) => { - if ( block.name === 'core/heading' ) { - return { - ...block, - path, - headingId: block.clientId, - level: block.attributes.level, - isEmpty: isEmptyHeading( block ), - }; - } - return computeOutlineHeadings( block.innerBlocks, [ ...path, block ] ); - } ); -}; - -const isEmptyHeading = ( heading ) => ! heading.attributes.content || heading.attributes.content.length === 0; + * Internal dependencies + */ +import { computeOutlineHeadings } from '../../../editor/src/components/document-outline/index.js'; export const HeadingChecker = ( { blocks = [], selectedLevel, selectedHeadingId } ) => { const headings = computeOutlineHeadings( blocks ); @@ -49,7 +20,7 @@ export const HeadingChecker = ( { blocks = [], selectedLevel, selectedHeadingId let prevHeadingLevel = 1; let i = 0; for ( i = 0; i < headings.length; i++ ) { - if ( i >= 1 && headings[ i ].headingId === selectedHeadingId ) { + if ( i >= 1 && headings[ i ].clientId === selectedHeadingId ) { prevHeadingLevel = headings[ i - 1 ].level; } } @@ -60,13 +31,7 @@ export const HeadingChecker = ( { blocks = [], selectedLevel, selectedHeadingId return null; } - // The correct heading level should be within [H2, prevHeadingLevel+1] - let suggestedHeading = 'H2'; - for ( i = 3; i < prevHeadingLevel + 2; i++ ) { - suggestedHeading = suggestedHeading + ', H' + i; - } - - const msg = __( 'This heading level is incorrect. Please use ' + suggestedHeading + ' instead.' ); + const msg = __( 'This heading level is incorrect. ' ); // For accessibility useEffect( () => { diff --git a/packages/editor/src/components/document-outline/index.js b/packages/editor/src/components/document-outline/index.js index 5a61503909a88b..d80d2ee8664d3b 100644 --- a/packages/editor/src/components/document-outline/index.js +++ b/packages/editor/src/components/document-outline/index.js @@ -48,7 +48,7 @@ const multipleH1Headings = [ * * @return {Array} An array of heading blocks enhanced with the properties described above. */ -const computeOutlineHeadings = ( blocks = [], path = [] ) => { +export const computeOutlineHeadings = ( blocks = [], path = [] ) => { return flatMap( blocks, ( block = {} ) => { if ( block.name === 'core/heading' ) { return { From df75f91c79e79804c299e76fa107011f900223fd Mon Sep 17 00:00:00 2001 From: Jackie6 <541172791@qq.com> Date: Thu, 11 Apr 2019 11:09:31 -0400 Subject: [PATCH 3/6] Change the way to import computeOutlineHeadings --- .../src/heading/heading-checker.js | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/heading/heading-checker.js b/packages/block-library/src/heading/heading-checker.js index 3bf3e9fa257349..cdd1330ad316c5 100644 --- a/packages/block-library/src/heading/heading-checker.js +++ b/packages/block-library/src/heading/heading-checker.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import { flatMap } from 'lodash'; + /** * WordPress dependencies */ @@ -8,10 +13,31 @@ import { useEffect } from '@wordpress/element'; import { compose } from '@wordpress/compose'; import { withSelect } from '@wordpress/data'; +// copy from packages/editor/src/components/document-outline/index.js /** - * Internal dependencies + * Returns an array of heading blocks enhanced with the following properties: + * path - An array of blocks that are ancestors of the heading starting from a top-level node. + * Can be an empty array if the heading is a top-level node (is not nested inside another block). + * level - An integer with the heading level. + * isEmpty - Flag indicating if the heading has no content. + * + * @param {?Array} blocks An array of blocks. + * @param {?Array} path An array of blocks that are ancestors of the blocks passed as blocks. + * + * @return {Array} An array of heading blocks enhanced with the properties described above. */ -import { computeOutlineHeadings } from '../../../editor/src/components/document-outline/index.js'; +export const computeOutlineHeadings = ( blocks = [], path = [] ) => { + return flatMap( blocks, ( block = {} ) => { + if ( block.name === 'core/heading' ) { + return { + ...block, + path, + level: block.attributes.level, + }; + } + return computeOutlineHeadings( block.innerBlocks, [ ...path, block ] ); + } ); +}; export const HeadingChecker = ( { blocks = [], selectedLevel, selectedHeadingId } ) => { const headings = computeOutlineHeadings( blocks ); From e99789215b48baa8dcab6a9e6f0e88a69617a736 Mon Sep 17 00:00:00 2001 From: Jackie6 <541172791@qq.com> Date: Thu, 11 Apr 2019 11:14:32 -0400 Subject: [PATCH 4/6] Remove selectedLevel passed to HeadingChecker --- packages/block-library/src/heading/edit.js | 2 +- packages/block-library/src/heading/heading-checker.js | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index c92bb339b7b8f8..20f5e968744113 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -39,7 +39,7 @@ export default function HeadingEdit( {

{ __( 'Level' ) }

setAttributes( { level: newLevel } ) } /> - +

{ __( 'Text Alignment' ) }

{ } ); }; -export const HeadingChecker = ( { blocks = [], selectedLevel, selectedHeadingId } ) => { +export const HeadingChecker = ( { blocks = [], selectedHeadingId } ) => { const headings = computeOutlineHeadings( blocks ); - // Iterate headings to find prevHeadingLevel + // Iterate headings to find prevHeadingLevel and selectedLevel let prevHeadingLevel = 1; + let selectedLevel = 1; let i = 0; for ( i = 0; i < headings.length; i++ ) { if ( i >= 1 && headings[ i ].clientId === selectedHeadingId ) { + selectedLevel = headings[ i ].level; prevHeadingLevel = headings[ i - 1 ].level; } } From c2dcb6b2fa264d51a15ff0aeedc53e17f69b9108 Mon Sep 17 00:00:00 2001 From: Jackie6 <541172791@qq.com> Date: Thu, 11 Apr 2019 11:29:08 -0400 Subject: [PATCH 5/6] Rename HeadingChecker --- packages/block-library/src/heading/edit.js | 4 ++-- .../heading/{heading-checker.js => level-checker.js} | 12 +++++++----- packages/block-library/src/heading/style.native.scss | 3 +-- 3 files changed, 10 insertions(+), 9 deletions(-) rename packages/block-library/src/heading/{heading-checker.js => level-checker.js} (88%) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index 20f5e968744113..90f8ddc8a071ad 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -2,7 +2,7 @@ * Internal dependencies */ import HeadingToolbar from './heading-toolbar'; -import HeadingChecker from './heading-checker'; +import HeadingLevelChecker from './level-checker'; /** * WordPress dependencies @@ -39,7 +39,7 @@ export default function HeadingEdit( {

{ __( 'Level' ) }

setAttributes( { level: newLevel } ) } /> - +

{ __( 'Text Alignment' ) }

{ } ); }; -export const HeadingChecker = ( { blocks = [], selectedHeadingId } ) => { +export const HeadingLevelChecker = ( { blocks = [], selectedHeadingId } ) => { const headings = computeOutlineHeadings( blocks ); // Iterate headings to find prevHeadingLevel and selectedLevel @@ -47,9 +47,11 @@ export const HeadingChecker = ( { blocks = [], selectedHeadingId } ) => { let selectedLevel = 1; let i = 0; for ( i = 0; i < headings.length; i++ ) { - if ( i >= 1 && headings[ i ].clientId === selectedHeadingId ) { + if ( headings[ i ].clientId === selectedHeadingId ) { selectedLevel = headings[ i ].level; - prevHeadingLevel = headings[ i - 1 ].level; + if ( i >= 1 ) { + prevHeadingLevel = headings[ i - 1 ].level; + } } } @@ -67,7 +69,7 @@ export const HeadingChecker = ( { blocks = [], selectedHeadingId } ) => { }, [ selectedLevel ] ); return ( -
+
{ msg } @@ -83,4 +85,4 @@ export default compose( blocks: getBlocks(), }; } ) -)( HeadingChecker ); +)( HeadingLevelChecker ); diff --git a/packages/block-library/src/heading/style.native.scss b/packages/block-library/src/heading/style.native.scss index e1be684c71ee93..ae869bd7f8b037 100644 --- a/packages/block-library/src/heading/style.native.scss +++ b/packages/block-library/src/heading/style.native.scss @@ -2,7 +2,6 @@ .blockText { min-height: $min-height-heading; } -.editor-heading-checker -.block-editor-heading-checker { +.block-library-heading__heading-level-checker { margin: 0; } From 3fa384548b249fec809931e8f92de09e503e32fa Mon Sep 17 00:00:00 2001 From: Jackie6 <541172791@qq.com> Date: Mon, 15 Apr 2019 11:52:35 -0400 Subject: [PATCH 6/6] Add checking for duplicate headings --- .../src/heading/level-checker.js | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/heading/level-checker.js b/packages/block-library/src/heading/level-checker.js index 7960c2f86a184b..7cf73ecb5397ac 100644 --- a/packages/block-library/src/heading/level-checker.js +++ b/packages/block-library/src/heading/level-checker.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { flatMap } from 'lodash'; +import { countBy, flatMap, get } from 'lodash'; /** * WordPress dependencies @@ -39,7 +39,7 @@ export const computeOutlineHeadings = ( blocks = [], path = [] ) => { } ); }; -export const HeadingLevelChecker = ( { blocks = [], selectedHeadingId } ) => { +export const HeadingLevelChecker = ( { blocks = [], title, isTitleSupported, selectedHeadingId } ) => { const headings = computeOutlineHeadings( blocks ); // Iterate headings to find prevHeadingLevel and selectedLevel @@ -55,17 +55,26 @@ export const HeadingLevelChecker = ( { blocks = [], selectedHeadingId } ) => { } } - const isIncorrectLevel = ( selectedLevel === 1 || selectedLevel > prevHeadingLevel + 1 ); + const titleNode = document.querySelector( '.editor-post-title__input' ); + const hasTitle = isTitleSupported && title && titleNode; + const countByLevel = countBy( headings, 'level' ); + const hasMultipleH1 = countByLevel[ 1 ] > 1; + const isIncorrectLevel = selectedLevel > prevHeadingLevel + 1; - if ( ! isIncorrectLevel ) { + let msg = ''; + if ( isIncorrectLevel ) { + msg = __( 'This heading level is incorrect.' ); + } else if ( selectedLevel === 1 && hasMultipleH1 ) { + msg = __( 'Multiple H1 headings found.' ); + } else if ( selectedLevel === 1 && hasTitle && ! hasMultipleH1 ) { + msg = __( 'H1 is already used for the post title.' ); + } else { return null; } - const msg = __( 'This heading level is incorrect. ' ); - // For accessibility useEffect( () => { - speak( __( 'This heading level is incorrect' ) ); + speak( msg ); }, [ selectedLevel ] ); return ( @@ -80,9 +89,14 @@ export const HeadingLevelChecker = ( { blocks = [], selectedHeadingId } ) => { export default compose( withSelect( ( select ) => { const { getBlocks } = select( 'core/block-editor' ); + const { getEditedPostAttribute } = select( 'core/editor' ); + const { getPostType } = select( 'core' ); + const postType = getPostType( getEditedPostAttribute( 'type' ) ); return { blocks: getBlocks(), + title: getEditedPostAttribute( 'title' ), + isTitleSupported: get( postType, [ 'supports', 'title' ], false ), }; } ) )( HeadingLevelChecker );