From 5cb5c9a724b8a534d679cbd5306d94d0792b16c5 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Tue, 13 Apr 2021 16:18:48 +0300 Subject: [PATCH 01/48] auto-generate anchors for heading blocks --- .../src/heading/autogenerate-anchors.js | 211 ++++++++++++++++++ packages/block-library/src/heading/index.js | 5 +- 2 files changed, 215 insertions(+), 1 deletion(-) create mode 100644 packages/block-library/src/heading/autogenerate-anchors.js diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js new file mode 100644 index 0000000000000..d50933046d23a --- /dev/null +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -0,0 +1,211 @@ +/** + * External dependencies + */ +import { isEmpty, isNil } from 'lodash'; + +/** + * WordPress dependencies + */ +import { dispatch, select, subscribe } from '@wordpress/data'; + +// This dummy element is used to strip all markup in getTextWithoutMarkup below. +const dummyElement = document.createElement( 'div' ); + +/** + * Runs a function over all blocks, including nested blocks. + * + * @param {Object[]} blocks The blocks. + * @param {Function} callback The callback. + * + * @return {void} + */ +function recurseOverBlocks( blocks, callback ) { + for ( const block of blocks ) { + // eslint-disable-next-line callback-return + callback( block ); + if ( block.innerBlocks ) { + recurseOverBlocks( block.innerBlocks, callback ); + } + } +} + +/** + * Returns the text without markup. + * + * @param {string} text The text. + * + * @return {string} The text without markup. + */ +function getTextWithoutMarkup( text ) { + dummyElement.innerHTML = text; + return dummyElement.innerText; +} + +/** + * Generates an anchor. + * + * @param {Object} block The block. + * @param {string[]} knownAnchors The known anchors. + * @param {string[]} blocksThatWereNotHeadings The block client IDs that weren't headings in the previous state. + * @param {boolean} fillAllAnchors Whether or not all empty anchors should be filled. + * + * @return {string} The anchor. + */ +function generateAnchor( + block, + knownAnchors, + blocksThatWereNotHeadings, + fillAllAnchors +) { + // Gutenberg doesn't save empty strings. + // So when anchor isn't set for a heading that already has content set an empty string. + // However, if none of the headings have anchors, we should assume the page was old, and still give all headings an anchor. + if ( + isNil( block.attributes.anchor ) && + ! fillAllAnchors && + ! isEmpty( block.attributes.content ) && + ! blocksThatWereNotHeadings.includes( block.clientId ) + ) { + return ''; + } + + const slug = getTextWithoutMarkup( block.attributes.content ) + .toLowerCase() + // Replace all non-word characters with dashes. + .replace( /[^\w]+/g, '-' ) + // Remove leading and trailing dashes. + .replace( /^-+|-+$/g, '' ); + const baseAnchor = `wp-${ slug }`; + let anchor = baseAnchor; + let i = 0; + + while ( knownAnchors.includes( anchor ) ) { + i += 1; + anchor = baseAnchor + '-' + i; + } + + return anchor; +} + +/** + * Updates the anchor if required. + * + * @param {Object} block The block. + * @param {Object} knownHeadings The known headings. + * @param {string[]} knownAnchors The known anchors. + * @param {string[]} blocksThatWereNotHeadings The block client IDs that weren't headings in the previous state. + * @param {boolean} fillAllAnchors Whether or not all empty anchors should be filled. + * + * @return {string} The anchor. + */ +function maybeUpdateAnchor( + block, + knownHeadings, + knownAnchors, + blocksThatWereNotHeadings, + fillAllAnchors +) { + let anchor = block.attributes.anchor; + + // If the block was previously unknown or has changed content and the anchor is empty or was set by us. + if ( + ( ! knownHeadings[ block.clientId ] || + knownHeadings[ block.clientId ].content !== + block.attributes.content ) && + ( isNil( anchor ) || anchor.startsWith( 'wp-' ) ) + ) { + anchor = generateAnchor( + block, + knownAnchors, + blocksThatWereNotHeadings, + fillAllAnchors + ); + + if ( anchor !== block.attributes.anchor ) { + dispatch( + 'core/block-editor' + ).updateBlockAttributes( block.clientId, { anchor } ); + } + } + + return anchor; +} + +/** + * Subscribes to the store to update blocks as they are added or suggestions are updated. + * + * @return {void} + */ +export default function subscribeToStore() { + let blockList = null; + let updatingHeadings = false; + let blocksThatWereNotHeadings = []; + const knownHeadings = {}; + + subscribe( () => { + if ( updatingHeadings ) { + return; + } + + const updatedBlockList = select( 'core/block-editor' ).getBlocks(); + const knownAnchors = []; + + // If there have been any change in the blocks. + if ( blockList !== updatedBlockList ) { + const headings = []; + const blocksThatAreNotHeadings = []; + updatingHeadings = true; + + /** + * Loop over all blocks and test whether all headings don't have anchors. + * If so, assume this is an older page. + */ + const headingAnchors = []; + recurseOverBlocks( updatedBlockList, ( block ) => { + if ( block.name === 'core/heading' ) { + headingAnchors.push( block.attributes.anchor ); + } + } ); + + // If all heading anchors are undefined, they should be populated. + const fillAllAnchors = headingAnchors.every( + ( anchor ) => undefined === anchor + ); + + // First loop over all core/heading blocks, give them anchors if necessary and collect all anchors. + recurseOverBlocks( updatedBlockList, ( block ) => { + if ( block.name === 'core/heading' ) { + const heading = block.attributes; + const content = getTextWithoutMarkup( heading.content ); + const anchor = maybeUpdateAnchor( + block, + knownHeadings, + knownAnchors, + blocksThatWereNotHeadings, + fillAllAnchors + ); + knownHeadings[ block.clientId ] = heading; + + // Empty strings shouldn't be added to the table of contents. + if ( anchor === '' || isEmpty( content ) ) { + return; + } + + knownAnchors.push( anchor ); + headings.push( { + content, + href: '#' + anchor, + level: heading.level, + } ); + } else { + blocksThatAreNotHeadings.push( block.clientId ); + } + } ); + + updatingHeadings = false; + blocksThatWereNotHeadings = blocksThatAreNotHeadings; + } + + blockList = updatedBlockList; + } ); +} diff --git a/packages/block-library/src/heading/index.js b/packages/block-library/src/heading/index.js index 593e1eaca7fe4..1a07f5ae05585 100644 --- a/packages/block-library/src/heading/index.js +++ b/packages/block-library/src/heading/index.js @@ -8,6 +8,7 @@ import { isEmpty } from 'lodash'; */ import { heading as icon } from '@wordpress/icons'; import { __, _x, sprintf } from '@wordpress/i18n'; +import domReady from '@wordpress/dom-ready'; /** * Internal dependencies @@ -17,9 +18,11 @@ import edit from './edit'; import metadata from './block.json'; import save from './save'; import transforms from './transforms'; - +import subscribeToStore from './autogenerate-anchors'; const { name } = metadata; +domReady( subscribeToStore ); + export { metadata, name }; export const settings = { From ca2f6646606abcbac197cc40c78981b7e5087451 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 14 Apr 2021 09:14:04 +0300 Subject: [PATCH 02/48] Don't use "we" & "us" in comments --- packages/block-library/src/heading/autogenerate-anchors.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index d50933046d23a..4054019c4d467 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -59,7 +59,7 @@ function generateAnchor( ) { // Gutenberg doesn't save empty strings. // So when anchor isn't set for a heading that already has content set an empty string. - // However, if none of the headings have anchors, we should assume the page was old, and still give all headings an anchor. + // However, if none of the headings have anchors, assume the page was old, and give all headings an anchor. if ( isNil( block.attributes.anchor ) && ! fillAllAnchors && @@ -107,7 +107,8 @@ function maybeUpdateAnchor( ) { let anchor = block.attributes.anchor; - // If the block was previously unknown or has changed content and the anchor is empty or was set by us. + // If the block was previously unknown or has changed content + // and the anchor is empty or was auto-generated. if ( ( ! knownHeadings[ block.clientId ] || knownHeadings[ block.clientId ].content !== From 556f82beac5ffa4021964ddcf70306fa87124ec6 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 14 Apr 2021 10:31:47 +0300 Subject: [PATCH 03/48] account for non-latin anchors --- .../src/heading/autogenerate-anchors.js | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index 4054019c4d467..9e0f67dcfa03f 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -1,12 +1,13 @@ /** * External dependencies */ -import { isEmpty, isNil } from 'lodash'; +import { isEmpty, isNil, deburr, trim } from 'lodash'; /** * WordPress dependencies */ import { dispatch, select, subscribe } from '@wordpress/data'; +import { cleanForSlug } from '@wordpress/url'; // This dummy element is used to strip all markup in getTextWithoutMarkup below. const dummyElement = document.createElement( 'div' ); @@ -69,12 +70,18 @@ function generateAnchor( return ''; } - const slug = getTextWithoutMarkup( block.attributes.content ) - .toLowerCase() - // Replace all non-word characters with dashes. - .replace( /[^\w]+/g, '-' ) - // Remove leading and trailing dashes. - .replace( /^-+|-+$/g, '' ); + // Get the slug. + let slug = cleanForSlug( block.attributes.content ); + // If slug is empty, then it's likely using non-latin characters. + if ( '' === slug ) { + slug = trim( + deburr( block.attributes.content ) + .replace( /[\s\./]+/g, '-' ) + .toLowerCase(), + '-' + ); + } + const baseAnchor = `wp-${ slug }`; let anchor = baseAnchor; let i = 0; From 4a6acd4b2de48325e242fc1cc1c041b80537859e Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 14 Apr 2021 12:39:39 +0300 Subject: [PATCH 04/48] remove lodash dependency --- .../src/heading/autogenerate-anchors.js | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index 9e0f67dcfa03f..d9a67f6dcff49 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -1,8 +1,3 @@ -/** - * External dependencies - */ -import { isEmpty, isNil, deburr, trim } from 'lodash'; - /** * WordPress dependencies */ @@ -62,9 +57,9 @@ function generateAnchor( // So when anchor isn't set for a heading that already has content set an empty string. // However, if none of the headings have anchors, assume the page was old, and give all headings an anchor. if ( - isNil( block.attributes.anchor ) && + ! block.attributes.anchor && ! fillAllAnchors && - ! isEmpty( block.attributes.content ) && + '' !== block.attributes.content.trim() && ! blocksThatWereNotHeadings.includes( block.clientId ) ) { return ''; @@ -74,12 +69,10 @@ function generateAnchor( let slug = cleanForSlug( block.attributes.content ); // If slug is empty, then it's likely using non-latin characters. if ( '' === slug ) { - slug = trim( - deburr( block.attributes.content ) - .replace( /[\s\./]+/g, '-' ) - .toLowerCase(), - '-' - ); + slug = block.attributes.content + .trim() + .replace( /[\s\./]+/g, '-' ) + .toLowerCase(); } const baseAnchor = `wp-${ slug }`; @@ -120,7 +113,7 @@ function maybeUpdateAnchor( ( ! knownHeadings[ block.clientId ] || knownHeadings[ block.clientId ].content !== block.attributes.content ) && - ( isNil( anchor ) || anchor.startsWith( 'wp-' ) ) + ( ! anchor || anchor.startsWith( 'wp-' ) ) ) { anchor = generateAnchor( block, @@ -195,7 +188,7 @@ export default function subscribeToStore() { knownHeadings[ block.clientId ] = heading; // Empty strings shouldn't be added to the table of contents. - if ( anchor === '' || isEmpty( content ) ) { + if ( anchor === '' || '' === content.trim() ) { return; } From 042a1aabcb1d765d8a5b0e91f3ec81756a0575c1 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 14 Apr 2021 12:39:39 +0300 Subject: [PATCH 05/48] Revert "remove lodash dependency" This reverts commit bf0517ca6029d874641f85b2087295ba0c42e5ba. --- .../src/heading/autogenerate-anchors.js | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index d9a67f6dcff49..9e0f67dcfa03f 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import { isEmpty, isNil, deburr, trim } from 'lodash'; + /** * WordPress dependencies */ @@ -57,9 +62,9 @@ function generateAnchor( // So when anchor isn't set for a heading that already has content set an empty string. // However, if none of the headings have anchors, assume the page was old, and give all headings an anchor. if ( - ! block.attributes.anchor && + isNil( block.attributes.anchor ) && ! fillAllAnchors && - '' !== block.attributes.content.trim() && + ! isEmpty( block.attributes.content ) && ! blocksThatWereNotHeadings.includes( block.clientId ) ) { return ''; @@ -69,10 +74,12 @@ function generateAnchor( let slug = cleanForSlug( block.attributes.content ); // If slug is empty, then it's likely using non-latin characters. if ( '' === slug ) { - slug = block.attributes.content - .trim() - .replace( /[\s\./]+/g, '-' ) - .toLowerCase(); + slug = trim( + deburr( block.attributes.content ) + .replace( /[\s\./]+/g, '-' ) + .toLowerCase(), + '-' + ); } const baseAnchor = `wp-${ slug }`; @@ -113,7 +120,7 @@ function maybeUpdateAnchor( ( ! knownHeadings[ block.clientId ] || knownHeadings[ block.clientId ].content !== block.attributes.content ) && - ( ! anchor || anchor.startsWith( 'wp-' ) ) + ( isNil( anchor ) || anchor.startsWith( 'wp-' ) ) ) { anchor = generateAnchor( block, @@ -188,7 +195,7 @@ export default function subscribeToStore() { knownHeadings[ block.clientId ] = heading; // Empty strings shouldn't be added to the table of contents. - if ( anchor === '' || '' === content.trim() ) { + if ( anchor === '' || isEmpty( content ) ) { return; } From 8c4be85ebe3f20f74f99785e63c6efe6f28db25a Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 14 Apr 2021 13:14:46 +0300 Subject: [PATCH 06/48] add dom-ready dependency --- packages/block-library/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/block-library/package.json b/packages/block-library/package.json index 03fb96d81fd90..06bd8347278ee 100644 --- a/packages/block-library/package.json +++ b/packages/block-library/package.json @@ -44,6 +44,7 @@ "@wordpress/date": "file:../date", "@wordpress/deprecated": "file:../deprecated", "@wordpress/dom": "file:../dom", + "@wordpress/dom-ready": "file:../dom-ready", "@wordpress/editor": "file:../editor", "@wordpress/element": "file:../element", "@wordpress/escape-html": "file:../escape-html", From 766ef32ed935a124bd014ba0a79b8e60a5a8c938 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Thu, 15 Apr 2021 12:24:23 +0300 Subject: [PATCH 07/48] refactor everything --- packages/block-library/package.json | 1 - .../src/heading/autogenerate-anchors.js | 201 +++++------------- packages/block-library/src/heading/edit.js | 12 ++ packages/block-library/src/heading/index.js | 4 - 4 files changed, 66 insertions(+), 152 deletions(-) diff --git a/packages/block-library/package.json b/packages/block-library/package.json index 06bd8347278ee..03fb96d81fd90 100644 --- a/packages/block-library/package.json +++ b/packages/block-library/package.json @@ -44,7 +44,6 @@ "@wordpress/date": "file:../date", "@wordpress/deprecated": "file:../deprecated", "@wordpress/dom": "file:../dom", - "@wordpress/dom-ready": "file:../dom-ready", "@wordpress/editor": "file:../editor", "@wordpress/element": "file:../element", "@wordpress/escape-html": "file:../escape-html", diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index 9e0f67dcfa03f..19de0b30998db 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -6,7 +6,7 @@ import { isEmpty, isNil, deburr, trim } from 'lodash'; /** * WordPress dependencies */ -import { dispatch, select, subscribe } from '@wordpress/data'; +import { select } from '@wordpress/data'; import { cleanForSlug } from '@wordpress/url'; // This dummy element is used to strip all markup in getTextWithoutMarkup below. @@ -20,7 +20,7 @@ const dummyElement = document.createElement( 'div' ); * * @return {void} */ -function recurseOverBlocks( blocks, callback ) { +const recurseOverBlocks = ( blocks, callback ) => { for ( const block of blocks ) { // eslint-disable-next-line callback-return callback( block ); @@ -28,7 +28,7 @@ function recurseOverBlocks( blocks, callback ) { recurseOverBlocks( block.innerBlocks, callback ); } } -} +}; /** * Returns the text without markup. @@ -37,45 +37,60 @@ function recurseOverBlocks( blocks, callback ) { * * @return {string} The text without markup. */ -function getTextWithoutMarkup( text ) { +const getTextWithoutMarkup = ( text ) => { dummyElement.innerHTML = text; return dummyElement.innerText; -} +}; /** - * Generates an anchor. + * Get all heading anchors. * - * @param {Object} block The block. - * @param {string[]} knownAnchors The known anchors. - * @param {string[]} blocksThatWereNotHeadings The block client IDs that weren't headings in the previous state. - * @param {boolean} fillAllAnchors Whether or not all empty anchors should be filled. + * @param {string} excludeId A block ID we want to exclude. * - * @return {string} The anchor. + * @return {string[]} Return an array of anchors. + */ +const getAllHeadingAnchors = ( excludeId ) => { + const blockList = select( 'core/block-editor' ).getBlocks(); + const anchors = []; + + recurseOverBlocks( blockList, ( block ) => { + if ( + block.name === 'core/heading' && + ( ! excludeId || block.clientId !== excludeId ) && + block.attributes.anchor + ) { + anchors.push( block.attributes.anchor ); + } + } ); + + return anchors; +}; + +// Whether this is an old post and we need to generate all headings or not. +const noAnchorsExist = isEmpty( getAllHeadingAnchors() ); + +/** + * Generate the anchor for a heading. + * + * @param {string} anchor The heading anchor. + * @param {string} content The block content. + * @param {clientId} clientId The block ID. + * + * @return {string} Return the heading anchor. */ -function generateAnchor( - block, - knownAnchors, - blocksThatWereNotHeadings, - fillAllAnchors -) { - // Gutenberg doesn't save empty strings. - // So when anchor isn't set for a heading that already has content set an empty string. - // However, if none of the headings have anchors, assume the page was old, and give all headings an anchor. - if ( - isNil( block.attributes.anchor ) && - ! fillAllAnchors && - ! isEmpty( block.attributes.content ) && - ! blocksThatWereNotHeadings.includes( block.clientId ) - ) { +const generateAnchor = ( anchor, content, clientId ) => { + content = getTextWithoutMarkup( content ); + // When anchor isn't set for a heading that already has content set an empty string. + if ( isNil( anchor ) && ! isEmpty( content ) ) { return ''; } // Get the slug. - let slug = cleanForSlug( block.attributes.content ); + let slug = cleanForSlug( content ); // If slug is empty, then it's likely using non-latin characters. if ( '' === slug ) { slug = trim( - deburr( block.attributes.content ) + deburr( content ) .replace( /[\s\./]+/g, '-' ) .toLowerCase(), '-' @@ -83,137 +98,29 @@ function generateAnchor( } const baseAnchor = `wp-${ slug }`; - let anchor = baseAnchor; + anchor = baseAnchor; let i = 0; - while ( knownAnchors.includes( anchor ) ) { + // If the anchor already exists in another heading, append -i. + while ( getAllHeadingAnchors( clientId ).includes( anchor ) ) { i += 1; anchor = baseAnchor + '-' + i; } return anchor; -} +}; /** * Updates the anchor if required. * - * @param {Object} block The block. - * @param {Object} knownHeadings The known headings. - * @param {string[]} knownAnchors The known anchors. - * @param {string[]} blocksThatWereNotHeadings The block client IDs that weren't headings in the previous state. - * @param {boolean} fillAllAnchors Whether or not all empty anchors should be filled. + * @param {string} anchor The heading anchor. + * @param {string} content The block content. + * @param {clientId} clientId The block ID. * * @return {string} The anchor. */ -function maybeUpdateAnchor( - block, - knownHeadings, - knownAnchors, - blocksThatWereNotHeadings, - fillAllAnchors -) { - let anchor = block.attributes.anchor; - - // If the block was previously unknown or has changed content - // and the anchor is empty or was auto-generated. - if ( - ( ! knownHeadings[ block.clientId ] || - knownHeadings[ block.clientId ].content !== - block.attributes.content ) && - ( isNil( anchor ) || anchor.startsWith( 'wp-' ) ) - ) { - anchor = generateAnchor( - block, - knownAnchors, - blocksThatWereNotHeadings, - fillAllAnchors - ); - - if ( anchor !== block.attributes.anchor ) { - dispatch( - 'core/block-editor' - ).updateBlockAttributes( block.clientId, { anchor } ); - } - } - - return anchor; -} - -/** - * Subscribes to the store to update blocks as they are added or suggestions are updated. - * - * @return {void} - */ -export default function subscribeToStore() { - let blockList = null; - let updatingHeadings = false; - let blocksThatWereNotHeadings = []; - const knownHeadings = {}; - - subscribe( () => { - if ( updatingHeadings ) { - return; - } - - const updatedBlockList = select( 'core/block-editor' ).getBlocks(); - const knownAnchors = []; - - // If there have been any change in the blocks. - if ( blockList !== updatedBlockList ) { - const headings = []; - const blocksThatAreNotHeadings = []; - updatingHeadings = true; - - /** - * Loop over all blocks and test whether all headings don't have anchors. - * If so, assume this is an older page. - */ - const headingAnchors = []; - recurseOverBlocks( updatedBlockList, ( block ) => { - if ( block.name === 'core/heading' ) { - headingAnchors.push( block.attributes.anchor ); - } - } ); - - // If all heading anchors are undefined, they should be populated. - const fillAllAnchors = headingAnchors.every( - ( anchor ) => undefined === anchor - ); - - // First loop over all core/heading blocks, give them anchors if necessary and collect all anchors. - recurseOverBlocks( updatedBlockList, ( block ) => { - if ( block.name === 'core/heading' ) { - const heading = block.attributes; - const content = getTextWithoutMarkup( heading.content ); - const anchor = maybeUpdateAnchor( - block, - knownHeadings, - knownAnchors, - blocksThatWereNotHeadings, - fillAllAnchors - ); - knownHeadings[ block.clientId ] = heading; - - // Empty strings shouldn't be added to the table of contents. - if ( anchor === '' || isEmpty( content ) ) { - return; - } - - knownAnchors.push( anchor ); - headings.push( { - content, - href: '#' + anchor, - level: heading.level, - } ); - } else { - blocksThatAreNotHeadings.push( block.clientId ); - } - } ); - - updatingHeadings = false; - blocksThatWereNotHeadings = blocksThatAreNotHeadings; - } - - blockList = updatedBlockList; - } ); +export default function maybeUpdateAnchor( anchor, content, clientId ) { + return noAnchorsExist || isNil( anchor ) || anchor.startsWith( 'wp-' ) + ? generateAnchor( anchor, content, clientId ) + : anchor; } diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index 0f8d900b62c4c..a65a00fa63a92 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -14,11 +14,13 @@ import { RichText, useBlockProps, } from '@wordpress/block-editor'; +import { useEffect } from '@wordpress/element'; /** * Internal dependencies */ import HeadingLevelDropdown from './heading-level-dropdown'; +import maybeUpdateAnchor from './autogenerate-anchors'; function HeadingEdit( { attributes, @@ -36,6 +38,16 @@ function HeadingEdit( { } ), style: mergedStyle, } ); + useEffect( () => { + const newAnchor = maybeUpdateAnchor( + attributes.anchor, + content, + clientId + ); + if ( newAnchor !== attributes.anchor ) { + setAttributes( { anchor: newAnchor } ); + } + }, [ attributes.anchor, content ] ); return ( <> diff --git a/packages/block-library/src/heading/index.js b/packages/block-library/src/heading/index.js index 1a07f5ae05585..5538a0fb71d84 100644 --- a/packages/block-library/src/heading/index.js +++ b/packages/block-library/src/heading/index.js @@ -8,7 +8,6 @@ import { isEmpty } from 'lodash'; */ import { heading as icon } from '@wordpress/icons'; import { __, _x, sprintf } from '@wordpress/i18n'; -import domReady from '@wordpress/dom-ready'; /** * Internal dependencies @@ -18,11 +17,8 @@ import edit from './edit'; import metadata from './block.json'; import save from './save'; import transforms from './transforms'; -import subscribeToStore from './autogenerate-anchors'; const { name } = metadata; -domReady( subscribeToStore ); - export { metadata, name }; export const settings = { From 329665158939e1bde01bc424bea1ad6a141769d8 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Fri, 16 Apr 2021 11:40:40 +0300 Subject: [PATCH 08/48] use useSelect --- .../src/heading/autogenerate-anchors.js | 40 ++++++++----------- packages/block-library/src/heading/edit.js | 12 +++++- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index 19de0b30998db..4c762d46707ff 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -1,19 +1,18 @@ /** * External dependencies */ -import { isEmpty, isNil, deburr, trim } from 'lodash'; +import { isNil, deburr, trim } from 'lodash'; /** * WordPress dependencies */ -import { select } from '@wordpress/data'; import { cleanForSlug } from '@wordpress/url'; // This dummy element is used to strip all markup in getTextWithoutMarkup below. const dummyElement = document.createElement( 'div' ); /** - * Runs a function over all blocks, including nested blocks. + * Runs a callback over all blocks, including nested blocks. * * @param {Object[]} blocks The blocks. * @param {Function} callback The callback. @@ -45,12 +44,12 @@ const getTextWithoutMarkup = ( text ) => { /** * Get all heading anchors. * + * @param {Object} blockList An object containing all blocks. * @param {string} excludeId A block ID we want to exclude. * * @return {string[]} Return an array of anchors. */ -const getAllHeadingAnchors = ( excludeId ) => { - const blockList = select( 'core/block-editor' ).getBlocks(); +export const getAllHeadingAnchors = ( blockList, excludeId ) => { const anchors = []; recurseOverBlocks( blockList, ( block ) => { @@ -66,24 +65,17 @@ const getAllHeadingAnchors = ( excludeId ) => { return anchors; }; -// Whether this is an old post and we need to generate all headings or not. -const noAnchorsExist = isEmpty( getAllHeadingAnchors() ); - /** * Generate the anchor for a heading. * - * @param {string} anchor The heading anchor. - * @param {string} content The block content. - * @param {clientId} clientId The block ID. + * @param {string} anchor The heading anchor. + * @param {string} content The block content. + * @param {string[]} allHeadingAnchors An array containing all headings anchors. * * @return {string} Return the heading anchor. */ -const generateAnchor = ( anchor, content, clientId ) => { +const generateAnchor = ( anchor, content, allHeadingAnchors ) => { content = getTextWithoutMarkup( content ); - // When anchor isn't set for a heading that already has content set an empty string. - if ( isNil( anchor ) && ! isEmpty( content ) ) { - return ''; - } // Get the slug. let slug = cleanForSlug( content ); @@ -102,7 +94,7 @@ const generateAnchor = ( anchor, content, clientId ) => { let i = 0; // If the anchor already exists in another heading, append -i. - while ( getAllHeadingAnchors( clientId ).includes( anchor ) ) { + while ( allHeadingAnchors.includes( anchor ) ) { i += 1; anchor = baseAnchor + '-' + i; } @@ -113,14 +105,14 @@ const generateAnchor = ( anchor, content, clientId ) => { /** * Updates the anchor if required. * - * @param {string} anchor The heading anchor. - * @param {string} content The block content. - * @param {clientId} clientId The block ID. + * @param {string} anchor The heading anchor. + * @param {string} content The block content. + * @param {string[]} allHeadingAnchors An array containing all headings anchors. * * @return {string} The anchor. */ -export default function maybeUpdateAnchor( anchor, content, clientId ) { - return noAnchorsExist || isNil( anchor ) || anchor.startsWith( 'wp-' ) - ? generateAnchor( anchor, content, clientId ) +export const maybeUpdateAnchor = ( anchor, content, allHeadingAnchors ) => { + return isNil( anchor ) || anchor.startsWith( 'wp-' ) + ? generateAnchor( anchor, content, allHeadingAnchors ) : anchor; -} +}; diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index a65a00fa63a92..420df82b580ba 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -15,12 +15,16 @@ import { useBlockProps, } from '@wordpress/block-editor'; import { useEffect } from '@wordpress/element'; +import { useSelect } from '@wordpress/data'; /** * Internal dependencies */ import HeadingLevelDropdown from './heading-level-dropdown'; -import maybeUpdateAnchor from './autogenerate-anchors'; +import { + getAllHeadingAnchors, + maybeUpdateAnchor, +} from './autogenerate-anchors'; function HeadingEdit( { attributes, @@ -38,11 +42,15 @@ function HeadingEdit( { } ), style: mergedStyle, } ); + const allHeadingAnchors = useSelect( ( select ) => { + const allBlocks = select( 'core/block-editor' ).getBlocks(); + return getAllHeadingAnchors( allBlocks, clientId ); + }, [] ); useEffect( () => { const newAnchor = maybeUpdateAnchor( attributes.anchor, content, - clientId + allHeadingAnchors ); if ( newAnchor !== attributes.anchor ) { setAttributes( { anchor: newAnchor } ); From 3e6928c91aac6cfc543879abf199b0751146d255 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Fri, 16 Apr 2021 11:44:42 +0300 Subject: [PATCH 09/48] This was not removed on purpose --- packages/block-library/src/heading/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/block-library/src/heading/index.js b/packages/block-library/src/heading/index.js index 5538a0fb71d84..593e1eaca7fe4 100644 --- a/packages/block-library/src/heading/index.js +++ b/packages/block-library/src/heading/index.js @@ -17,6 +17,7 @@ import edit from './edit'; import metadata from './block.json'; import save from './save'; import transforms from './transforms'; + const { name } = metadata; export { metadata, name }; From 1e5d3d7cc02b5a60616c0382ca850f033d30a93c Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Fri, 16 Apr 2021 12:32:01 +0300 Subject: [PATCH 10/48] Move dummyElement inside getTextWithoutMarkup --- packages/block-library/src/heading/autogenerate-anchors.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index 4c762d46707ff..327b0c3190990 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -8,9 +8,6 @@ import { isNil, deburr, trim } from 'lodash'; */ import { cleanForSlug } from '@wordpress/url'; -// This dummy element is used to strip all markup in getTextWithoutMarkup below. -const dummyElement = document.createElement( 'div' ); - /** * Runs a callback over all blocks, including nested blocks. * @@ -37,6 +34,7 @@ const recurseOverBlocks = ( blocks, callback ) => { * @return {string} The text without markup. */ const getTextWithoutMarkup = ( text ) => { + const dummyElement = document.createElement( 'div' ); dummyElement.innerHTML = text; return dummyElement.innerText; }; From 2d2013570d8ebd83ec5b505285f79eacad2d04d8 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Mon, 19 Apr 2021 12:17:46 +0300 Subject: [PATCH 11/48] Update e2e tests --- .../blocks/__snapshots__/heading.test.js.snap | 16 +++++++-------- .../blocks/__snapshots__/quote.test.js.snap | 20 +++++++++---------- .../__snapshots__/block-grouping.test.js.snap | 8 ++++---- .../inserting-blocks.test.js.snap | 4 ++-- .../specs/widgets/adding-widgets.test.js | 2 +- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/packages/e2e-tests/specs/editor/blocks/__snapshots__/heading.test.js.snap b/packages/e2e-tests/specs/editor/blocks/__snapshots__/heading.test.js.snap index c340a475f354a..aae12c11ca0d9 100644 --- a/packages/e2e-tests/specs/editor/blocks/__snapshots__/heading.test.js.snap +++ b/packages/e2e-tests/specs/editor/blocks/__snapshots__/heading.test.js.snap @@ -2,25 +2,25 @@ exports[`Heading can be created by prefixing existing content with number signs and a space 1`] = ` " -

4

+

4

" `; exports[`Heading can be created by prefixing number sign and a space 1`] = ` " -

3

+

3

" `; exports[`Heading should correctly apply custom colors 1`] = ` " -

Heading

+

Heading

" `; exports[`Heading should correctly apply named colors 1`] = ` " -

Heading

+

Heading

" `; @@ -30,13 +30,13 @@ exports[`Heading should create a paragraph block above when pressing enter at th -

a

+

a

" `; exports[`Heading should create a paragraph block below when pressing enter at the end 1`] = ` " -

a

+

a

@@ -46,12 +46,12 @@ exports[`Heading should create a paragraph block below when pressing enter at th exports[`Heading should not work with the list input rule 1`] = ` " -

1. H

+

1. H

" `; exports[`Heading should work with the format input rules 1`] = ` " -

code

+

code

" `; diff --git a/packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap b/packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap index 57fcc6c61dbdd..5b182f49bb5c4 100644 --- a/packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap +++ b/packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap @@ -40,7 +40,7 @@ exports[`Quote can be converted to paragraphs and renders only one paragraph for exports[`Quote can be created by converting a heading 1`] = ` " -

test

+

test

" `; @@ -144,7 +144,7 @@ exports[`Quote can be split in the middle and merged back 4`] = ` exports[`Quote is transformed to a heading and a quote if the quote contains a citation 1`] = ` " -

one

+

one

@@ -154,7 +154,7 @@ exports[`Quote is transformed to a heading and a quote if the quote contains a c exports[`Quote is transformed to a heading and a quote if the quote contains multiple paragraphs 1`] = ` " -

one

+

one

@@ -164,7 +164,7 @@ exports[`Quote is transformed to a heading and a quote if the quote contains mul exports[`Quote is transformed to a heading if the quote just contains one paragraph 1`] = ` " -

one

+

one

" `; @@ -176,7 +176,7 @@ exports[`Quote is transformed to an empty heading if the quote is empty 1`] = ` exports[`Quote the resuling quote after transforming to a heading can be transformed again 1`] = ` " -

one

+

one

@@ -186,11 +186,11 @@ exports[`Quote the resuling quote after transforming to a heading can be transfo exports[`Quote the resuling quote after transforming to a heading can be transformed again 2`] = ` " -

one

+

one

-

two

+

two

@@ -200,14 +200,14 @@ exports[`Quote the resuling quote after transforming to a heading can be transfo exports[`Quote the resuling quote after transforming to a heading can be transformed again 3`] = ` " -

one

+

one

-

two

+

two

-

cite

+

cite

" `; diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/block-grouping.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/block-grouping.test.js.snap index f3c24c200b971..62b4bebfc9914 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/block-grouping.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/block-grouping.test.js.snap @@ -3,7 +3,7 @@ exports[`Block Grouping Group creation creates a group from multiple blocks of different types via block transforms 1`] = ` "
-

Group Heading

+

Group Heading

@@ -51,7 +51,7 @@ exports[`Block Grouping Group creation creates a group from multiple blocks of t exports[`Block Grouping Group creation groups and ungroups multiple blocks of different types via options toolbar 1`] = ` "
-

Group Heading

+

Group Heading

@@ -66,7 +66,7 @@ exports[`Block Grouping Group creation groups and ungroups multiple blocks of di exports[`Block Grouping Group creation groups and ungroups multiple blocks of different types via options toolbar 2`] = ` " -

Group Heading

+

Group Heading

@@ -81,7 +81,7 @@ exports[`Block Grouping Group creation groups and ungroups multiple blocks of di exports[`Block Grouping Preserving selected blocks attributes preserves width alignment settings of selected blocks 1`] = ` "
-

Group Heading

+

Group Heading

diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/inserting-blocks.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/inserting-blocks.test.js.snap index ec465386aaf05..403eb377d3861 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/inserting-blocks.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/inserting-blocks.test.js.snap @@ -71,7 +71,7 @@ exports[`Inserting blocks inserts a block in proper place after having clicked \ -

Heading

+

Heading

@@ -93,7 +93,7 @@ exports[`Inserting blocks inserts a block in proper place after having clicked \ -

Heading

+

Heading

diff --git a/packages/e2e-tests/specs/widgets/adding-widgets.test.js b/packages/e2e-tests/specs/widgets/adding-widgets.test.js index 412e433ae3247..78d0362603f67 100644 --- a/packages/e2e-tests/specs/widgets/adding-widgets.test.js +++ b/packages/e2e-tests/specs/widgets/adding-widgets.test.js @@ -347,7 +347,7 @@ describe( 'Widgets screen', () => {

First Paragraph

-

My Heading

+

My Heading

Second Paragraph

From 69efbab3fe7ed687f94b10534b2cae905f10e22e Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Mon, 19 Apr 2021 12:19:45 +0300 Subject: [PATCH 12/48] Don't use a plain "wp-" as anchor --- packages/block-library/src/heading/autogenerate-anchors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index 327b0c3190990..aa9f249010e8f 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -97,7 +97,7 @@ const generateAnchor = ( anchor, content, allHeadingAnchors ) => { anchor = baseAnchor + '-' + i; } - return anchor; + return 'wp-' === anchor ? '' : anchor; }; /** From e5af9c81d6776d9c5abbec0467f02380a73e7f4b Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Mon, 19 Apr 2021 12:22:46 +0300 Subject: [PATCH 13/48] typo --- packages/e2e-tests/specs/widgets/adding-widgets.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/widgets/adding-widgets.test.js b/packages/e2e-tests/specs/widgets/adding-widgets.test.js index 78d0362603f67..b19097cff18f5 100644 --- a/packages/e2e-tests/specs/widgets/adding-widgets.test.js +++ b/packages/e2e-tests/specs/widgets/adding-widgets.test.js @@ -347,7 +347,7 @@ describe( 'Widgets screen', () => {

First Paragraph

-

My Heading

+

My Heading

Second Paragraph

From 15e04db3c4d5bb53d308276c21e2fe189113fe65 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Mon, 19 Apr 2021 12:39:55 +0300 Subject: [PATCH 14/48] Improve empty string handling --- .../block-library/src/heading/autogenerate-anchors.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index aa9f249010e8f..1c773b0eafea7 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -77,7 +77,8 @@ const generateAnchor = ( anchor, content, allHeadingAnchors ) => { // Get the slug. let slug = cleanForSlug( content ); - // If slug is empty, then it's likely using non-latin characters. + // If slug is empty, then it's either empty, or using non-latin characters. + // Try for non-latin first. if ( '' === slug ) { slug = trim( deburr( content ) @@ -86,6 +87,10 @@ const generateAnchor = ( anchor, content, allHeadingAnchors ) => { '-' ); } + // If slug is still empty, then return empty strng. + if ( '' === slug ) { + return ''; + } const baseAnchor = `wp-${ slug }`; anchor = baseAnchor; @@ -97,7 +102,7 @@ const generateAnchor = ( anchor, content, allHeadingAnchors ) => { anchor = baseAnchor + '-' + i; } - return 'wp-' === anchor ? '' : anchor; + return anchor; }; /** From 635eca9f349129ffd84cf5d6ff07d164d5b052a4 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Mon, 19 Apr 2021 12:42:36 +0300 Subject: [PATCH 15/48] Improve inline comments --- packages/block-library/src/heading/autogenerate-anchors.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index 1c773b0eafea7..cd21b096a0783 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -77,8 +77,8 @@ const generateAnchor = ( anchor, content, allHeadingAnchors ) => { // Get the slug. let slug = cleanForSlug( content ); - // If slug is empty, then it's either empty, or using non-latin characters. - // Try for non-latin first. + // If slug is empty, then there is no content, or content is using non-latin characters. + // Try non-latin first. if ( '' === slug ) { slug = trim( deburr( content ) @@ -87,7 +87,7 @@ const generateAnchor = ( anchor, content, allHeadingAnchors ) => { '-' ); } - // If slug is still empty, then return empty strng. + // If slug is still empty, then return empty string. if ( '' === slug ) { return ''; } From 4c03e1b1c930b1e199552b0c647d867669e88309 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Mon, 19 Apr 2021 13:41:57 +0300 Subject: [PATCH 16/48] Bugfix anchor generation for empty headings --- .../src/heading/autogenerate-anchors.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index cd21b096a0783..8075aba019acc 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { isNil, deburr, trim } from 'lodash'; +import { isNil, deburr, trim, startsWith } from 'lodash'; /** * WordPress dependencies @@ -70,7 +70,7 @@ export const getAllHeadingAnchors = ( blockList, excludeId ) => { * @param {string} content The block content. * @param {string[]} allHeadingAnchors An array containing all headings anchors. * - * @return {string} Return the heading anchor. + * @return {string|null} Return the heading anchor. */ const generateAnchor = ( anchor, content, allHeadingAnchors ) => { content = getTextWithoutMarkup( content ); @@ -87,9 +87,10 @@ const generateAnchor = ( anchor, content, allHeadingAnchors ) => { '-' ); } - // If slug is still empty, then return empty string. + // If slug is still empty, then return null. + // Returning null instead of an empty string allows us to check again when the content changes. if ( '' === slug ) { - return ''; + return null; } const baseAnchor = `wp-${ slug }`; @@ -115,7 +116,8 @@ const generateAnchor = ( anchor, content, allHeadingAnchors ) => { * @return {string} The anchor. */ export const maybeUpdateAnchor = ( anchor, content, allHeadingAnchors ) => { - return isNil( anchor ) || anchor.startsWith( 'wp-' ) - ? generateAnchor( anchor, content, allHeadingAnchors ) - : anchor; + if ( isNil( anchor ) || startsWith( anchor, 'wp-' ) ) { + return generateAnchor( anchor, content, allHeadingAnchors ); + } + return anchor; }; From abeaaaf17f49dfc521fb6d7b3b06c17eecedf6d7 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Tue, 20 Apr 2021 09:32:15 +0300 Subject: [PATCH 17/48] Update packages/block-library/src/heading/edit.js Co-authored-by: Nik Tsekouras --- packages/block-library/src/heading/edit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index 420df82b580ba..5f2ebf6d73aff 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -45,7 +45,7 @@ function HeadingEdit( { const allHeadingAnchors = useSelect( ( select ) => { const allBlocks = select( 'core/block-editor' ).getBlocks(); return getAllHeadingAnchors( allBlocks, clientId ); - }, [] ); + }, [ clientId ] ); useEffect( () => { const newAnchor = maybeUpdateAnchor( attributes.anchor, From 9f728c24cebd023a7b1f7dc8172c3f5c762266d3 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Tue, 20 Apr 2021 09:39:37 +0300 Subject: [PATCH 18/48] Use blockEditorStore --- packages/block-library/src/heading/edit.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index 5f2ebf6d73aff..0727c6f988966 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -13,6 +13,7 @@ import { BlockControls, RichText, useBlockProps, + store as blockEditorStore } from '@wordpress/block-editor'; import { useEffect } from '@wordpress/element'; import { useSelect } from '@wordpress/data'; @@ -43,7 +44,7 @@ function HeadingEdit( { style: mergedStyle, } ); const allHeadingAnchors = useSelect( ( select ) => { - const allBlocks = select( 'core/block-editor' ).getBlocks(); + const allBlocks = select( blockEditorStore ).getBlocks(); return getAllHeadingAnchors( allBlocks, clientId ); }, [ clientId ] ); useEffect( () => { From 4579a725ac981ef64c2a0facf526ce20a734cc67 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Tue, 20 Apr 2021 09:40:10 +0300 Subject: [PATCH 19/48] cs --- packages/block-library/src/heading/edit.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index 0727c6f988966..b24213d5c2896 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -13,7 +13,7 @@ import { BlockControls, RichText, useBlockProps, - store as blockEditorStore + store as blockEditorStore, } from '@wordpress/block-editor'; import { useEffect } from '@wordpress/element'; import { useSelect } from '@wordpress/data'; @@ -43,10 +43,13 @@ function HeadingEdit( { } ), style: mergedStyle, } ); - const allHeadingAnchors = useSelect( ( select ) => { - const allBlocks = select( blockEditorStore ).getBlocks(); - return getAllHeadingAnchors( allBlocks, clientId ); - }, [ clientId ] ); + const allHeadingAnchors = useSelect( + ( select ) => { + const allBlocks = select( blockEditorStore ).getBlocks(); + return getAllHeadingAnchors( allBlocks, clientId ); + }, + [ clientId ] + ); useEffect( () => { const newAnchor = maybeUpdateAnchor( attributes.anchor, From 93b0d4dea749bf3f3e41e204d59267ee42637f6b Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Tue, 20 Apr 2021 10:03:34 +0300 Subject: [PATCH 20/48] refactor hook --- .../src/heading/autogenerate-anchors.js | 21 ++++++++++----- packages/block-library/src/heading/edit.js | 26 +++++-------------- 2 files changed, 22 insertions(+), 25 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index 8075aba019acc..6a8d08ea5becd 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -7,6 +7,8 @@ import { isNil, deburr, trim, startsWith } from 'lodash'; * WordPress dependencies */ import { cleanForSlug } from '@wordpress/url'; +import { useSelect } from '@wordpress/data'; +import { store as blockEditorStore } from '@wordpress/block-editor'; /** * Runs a callback over all blocks, including nested blocks. @@ -47,7 +49,7 @@ const getTextWithoutMarkup = ( text ) => { * * @return {string[]} Return an array of anchors. */ -export const getAllHeadingAnchors = ( blockList, excludeId ) => { +const getAllHeadingAnchors = ( blockList, excludeId ) => { const anchors = []; recurseOverBlocks( blockList, ( block ) => { @@ -109,15 +111,22 @@ const generateAnchor = ( anchor, content, allHeadingAnchors ) => { /** * Updates the anchor if required. * - * @param {string} anchor The heading anchor. - * @param {string} content The block content. - * @param {string[]} allHeadingAnchors An array containing all headings anchors. + * @param {string} clientId The block's client-ID. + * @param {string} anchor The heading anchor. + * @param {string} content The block content. * * @return {string} The anchor. */ -export const maybeUpdateAnchor = ( anchor, content, allHeadingAnchors ) => { +export default function useGeneratedAnchor( clientId, anchor, content ) { + const allHeadingAnchors = useSelect( + ( select ) => { + const allBlocks = select( blockEditorStore ).getBlocks(); + return getAllHeadingAnchors( allBlocks, clientId ); + }, + [ clientId ] + ); if ( isNil( anchor ) || startsWith( anchor, 'wp-' ) ) { return generateAnchor( anchor, content, allHeadingAnchors ); } return anchor; -}; +} diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index b24213d5c2896..d2037a27e65a2 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -13,19 +13,14 @@ import { BlockControls, RichText, useBlockProps, - store as blockEditorStore, } from '@wordpress/block-editor'; import { useEffect } from '@wordpress/element'; -import { useSelect } from '@wordpress/data'; /** * Internal dependencies */ import HeadingLevelDropdown from './heading-level-dropdown'; -import { - getAllHeadingAnchors, - maybeUpdateAnchor, -} from './autogenerate-anchors'; +import useGeneratedAnchor from './autogenerate-anchors'; function HeadingEdit( { attributes, @@ -43,21 +38,14 @@ function HeadingEdit( { } ), style: mergedStyle, } ); - const allHeadingAnchors = useSelect( - ( select ) => { - const allBlocks = select( blockEditorStore ).getBlocks(); - return getAllHeadingAnchors( allBlocks, clientId ); - }, - [ clientId ] + const generatedAnchor = useGeneratedAnchor( + clientId, + attributes.anchor, + content ); useEffect( () => { - const newAnchor = maybeUpdateAnchor( - attributes.anchor, - content, - allHeadingAnchors - ); - if ( newAnchor !== attributes.anchor ) { - setAttributes( { anchor: newAnchor } ); + if ( generatedAnchor !== attributes.anchor ) { + setAttributes( { anchor: generatedAnchor } ); } }, [ attributes.anchor, content ] ); From a9f6381cf26f49397467830784f66134a4fc4a41 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Thu, 22 Apr 2021 17:01:48 +0300 Subject: [PATCH 21/48] Update packages/block-library/src/heading/edit.js Co-authored-by: Nik Tsekouras --- packages/block-library/src/heading/edit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index d2037a27e65a2..ebbe733686f8e 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -47,7 +47,7 @@ function HeadingEdit( { if ( generatedAnchor !== attributes.anchor ) { setAttributes( { anchor: generatedAnchor } ); } - }, [ attributes.anchor, content ] ); + }, [ attributes.anchor, generatedAnchor ] ); return ( <> From 762af2c3a5fa602227b3601910ebad89dce8a349 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Thu, 22 Apr 2021 17:07:30 +0300 Subject: [PATCH 22/48] no need for lodash here, these are pretty simple --- packages/block-library/src/heading/autogenerate-anchors.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index 6a8d08ea5becd..799e6026f6bdd 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { isNil, deburr, trim, startsWith } from 'lodash'; +import { deburr, trim } from 'lodash'; /** * WordPress dependencies @@ -125,7 +125,8 @@ export default function useGeneratedAnchor( clientId, anchor, content ) { }, [ clientId ] ); - if ( isNil( anchor ) || startsWith( anchor, 'wp-' ) ) { + // eslint-disable-next-line eqeqeq + if ( null == anchor || 0 === anchor.indexOf( 'wp-' ) ) { return generateAnchor( anchor, content, allHeadingAnchors ); } return anchor; From c3ccf1ce5285e2ca284505850277b8a086213e15 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Thu, 22 Apr 2021 17:57:46 +0300 Subject: [PATCH 23/48] refactor - no prefix --- .../src/heading/autogenerate-anchors.js | 36 ++++++++++++------- packages/block-library/src/heading/edit.js | 22 ++++++++++-- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index 799e6026f6bdd..fda20fe44dcee 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -66,19 +66,18 @@ const getAllHeadingAnchors = ( blockList, excludeId ) => { }; /** - * Generate the anchor for a heading. + * Get the slug from the content. * - * @param {string} anchor The heading anchor. - * @param {string} content The block content. - * @param {string[]} allHeadingAnchors An array containing all headings anchors. + * @param {string} content The block content. * - * @return {string|null} Return the heading anchor. + * @return {string} Returns the slug. */ -const generateAnchor = ( anchor, content, allHeadingAnchors ) => { +const getSlug = ( content ) => { content = getTextWithoutMarkup( content ); // Get the slug. let slug = cleanForSlug( content ); + // If slug is empty, then there is no content, or content is using non-latin characters. // Try non-latin first. if ( '' === slug ) { @@ -89,13 +88,28 @@ const generateAnchor = ( anchor, content, allHeadingAnchors ) => { '-' ); } - // If slug is still empty, then return null. + + return slug; +}; + +/** + * Generate the anchor for a heading. + * + * @param {string} anchor The heading anchor. + * @param {string} content The block content. + * @param {string[]} allHeadingAnchors An array containing all headings anchors. + * + * @return {string|null} Return the heading anchor. + */ +const generateAnchor = ( anchor, content, allHeadingAnchors ) => { + const slug = getSlug( content ); + // If slug is empty, then return null. // Returning null instead of an empty string allows us to check again when the content changes. if ( '' === slug ) { return null; } - const baseAnchor = `wp-${ slug }`; + const baseAnchor = slug; anchor = baseAnchor; let i = 0; @@ -125,9 +139,5 @@ export default function useGeneratedAnchor( clientId, anchor, content ) { }, [ clientId ] ); - // eslint-disable-next-line eqeqeq - if ( null == anchor || 0 === anchor.indexOf( 'wp-' ) ) { - return generateAnchor( anchor, content, allHeadingAnchors ); - } - return anchor; + return generateAnchor( anchor, content, allHeadingAnchors ); } diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index ebbe733686f8e..f591eb1e979b5 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -43,11 +43,29 @@ function HeadingEdit( { attributes.anchor, content ); + + // Set the isAnchorAutoGenerated attribute. + // Please note that this attribute does not get saved. + if ( generatedAnchor === attributes.anchor ) { + setAttributes( { isAnchorAutoGenerated: true } ); + } + + // Handle content changes. useEffect( () => { - if ( generatedAnchor !== attributes.anchor ) { + if ( + attributes.isAnchorAutoGenerated && + generatedAnchor !== attributes.anchor + ) { setAttributes( { anchor: generatedAnchor } ); } - }, [ attributes.anchor, generatedAnchor ] ); + }, [ generatedAnchor ] ); + + // Handle anchor changes. + useEffect( () => { + setAttributes( { + isAnchorAutoGenerated: generatedAnchor === attributes.anchor, + } ); + }, [ attributes.anchor ] ); return ( <> From 47014ddef462be5ab76959a46d6611c9de901301 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Fri, 23 Apr 2021 08:47:23 +0300 Subject: [PATCH 24/48] update e2e --- .../blocks/__snapshots__/heading.test.js.snap | 16 +++++++-------- .../blocks/__snapshots__/quote.test.js.snap | 20 +++++++++---------- .../__snapshots__/block-grouping.test.js.snap | 8 ++++---- .../inserting-blocks.test.js.snap | 4 ++-- .../specs/widgets/adding-widgets.test.js | 2 +- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/packages/e2e-tests/specs/editor/blocks/__snapshots__/heading.test.js.snap b/packages/e2e-tests/specs/editor/blocks/__snapshots__/heading.test.js.snap index aae12c11ca0d9..290019372feeb 100644 --- a/packages/e2e-tests/specs/editor/blocks/__snapshots__/heading.test.js.snap +++ b/packages/e2e-tests/specs/editor/blocks/__snapshots__/heading.test.js.snap @@ -2,25 +2,25 @@ exports[`Heading can be created by prefixing existing content with number signs and a space 1`] = ` " -

4

+

4

" `; exports[`Heading can be created by prefixing number sign and a space 1`] = ` " -

3

+

3

" `; exports[`Heading should correctly apply custom colors 1`] = ` " -

Heading

+

Heading

" `; exports[`Heading should correctly apply named colors 1`] = ` " -

Heading

+

Heading

" `; @@ -30,13 +30,13 @@ exports[`Heading should create a paragraph block above when pressing enter at th -

a

+

a

" `; exports[`Heading should create a paragraph block below when pressing enter at the end 1`] = ` " -

a

+

a

@@ -46,12 +46,12 @@ exports[`Heading should create a paragraph block below when pressing enter at th exports[`Heading should not work with the list input rule 1`] = ` " -

1. H

+

1. H

" `; exports[`Heading should work with the format input rules 1`] = ` " -

code

+

code

" `; diff --git a/packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap b/packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap index 5b182f49bb5c4..6345efae5ed72 100644 --- a/packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap +++ b/packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap @@ -40,7 +40,7 @@ exports[`Quote can be converted to paragraphs and renders only one paragraph for exports[`Quote can be created by converting a heading 1`] = ` " -

test

+

test

" `; @@ -144,7 +144,7 @@ exports[`Quote can be split in the middle and merged back 4`] = ` exports[`Quote is transformed to a heading and a quote if the quote contains a citation 1`] = ` " -

one

+

one

@@ -154,7 +154,7 @@ exports[`Quote is transformed to a heading and a quote if the quote contains a c exports[`Quote is transformed to a heading and a quote if the quote contains multiple paragraphs 1`] = ` " -

one

+

one

@@ -164,7 +164,7 @@ exports[`Quote is transformed to a heading and a quote if the quote contains mul exports[`Quote is transformed to a heading if the quote just contains one paragraph 1`] = ` " -

one

+

one

" `; @@ -176,7 +176,7 @@ exports[`Quote is transformed to an empty heading if the quote is empty 1`] = ` exports[`Quote the resuling quote after transforming to a heading can be transformed again 1`] = ` " -

one

+

one

@@ -186,11 +186,11 @@ exports[`Quote the resuling quote after transforming to a heading can be transfo exports[`Quote the resuling quote after transforming to a heading can be transformed again 2`] = ` " -

one

+

one

-

two

+

two

@@ -200,14 +200,14 @@ exports[`Quote the resuling quote after transforming to a heading can be transfo exports[`Quote the resuling quote after transforming to a heading can be transformed again 3`] = ` " -

one

+

one

-

two

+

two

-

cite

+

cite

" `; diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/block-grouping.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/block-grouping.test.js.snap index 62b4bebfc9914..aeaafed4ea8a7 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/block-grouping.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/block-grouping.test.js.snap @@ -3,7 +3,7 @@ exports[`Block Grouping Group creation creates a group from multiple blocks of different types via block transforms 1`] = ` "
-

Group Heading

+

Group Heading

@@ -51,7 +51,7 @@ exports[`Block Grouping Group creation creates a group from multiple blocks of t exports[`Block Grouping Group creation groups and ungroups multiple blocks of different types via options toolbar 1`] = ` "
-

Group Heading

+

Group Heading

@@ -66,7 +66,7 @@ exports[`Block Grouping Group creation groups and ungroups multiple blocks of di exports[`Block Grouping Group creation groups and ungroups multiple blocks of different types via options toolbar 2`] = ` " -

Group Heading

+

Group Heading

@@ -81,7 +81,7 @@ exports[`Block Grouping Group creation groups and ungroups multiple blocks of di exports[`Block Grouping Preserving selected blocks attributes preserves width alignment settings of selected blocks 1`] = ` "
-

Group Heading

+

Group Heading

diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/inserting-blocks.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/inserting-blocks.test.js.snap index 403eb377d3861..b97b80ddf59ae 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/inserting-blocks.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/inserting-blocks.test.js.snap @@ -71,7 +71,7 @@ exports[`Inserting blocks inserts a block in proper place after having clicked \ -

Heading

+

Heading

@@ -93,7 +93,7 @@ exports[`Inserting blocks inserts a block in proper place after having clicked \ -

Heading

+

Heading

diff --git a/packages/e2e-tests/specs/widgets/adding-widgets.test.js b/packages/e2e-tests/specs/widgets/adding-widgets.test.js index b19097cff18f5..2fa6a61ab951f 100644 --- a/packages/e2e-tests/specs/widgets/adding-widgets.test.js +++ b/packages/e2e-tests/specs/widgets/adding-widgets.test.js @@ -347,7 +347,7 @@ describe( 'Widgets screen', () => {

First Paragraph

-

My Heading

+

My Heading

Second Paragraph

From cc349944c626b2fa86ae79c58d5cb9a642b0f0f7 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Fri, 23 Apr 2021 10:11:11 +0300 Subject: [PATCH 25/48] bugfix --- packages/block-library/src/heading/edit.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index f591eb1e979b5..c874c3400275b 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -46,7 +46,10 @@ function HeadingEdit( { // Set the isAnchorAutoGenerated attribute. // Please note that this attribute does not get saved. - if ( generatedAnchor === attributes.anchor ) { + if ( + generatedAnchor === attributes.anchor || + ( undefined === attributes.anchor && null === generatedAnchor ) + ) { setAttributes( { isAnchorAutoGenerated: true } ); } From 4b9e0376bd2e11a41109636c0ff89e8f3207edff Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Fri, 23 Apr 2021 10:11:59 +0300 Subject: [PATCH 26/48] simplify --- packages/block-library/src/heading/edit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index c874c3400275b..0308d1d07e8aa 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -48,7 +48,7 @@ function HeadingEdit( { // Please note that this attribute does not get saved. if ( generatedAnchor === attributes.anchor || - ( undefined === attributes.anchor && null === generatedAnchor ) + ( ! attributes.anchor && ! generatedAnchor ) ) { setAttributes( { isAnchorAutoGenerated: true } ); } From a1263d9f986f739f98b817901a21985fa7ae7a02 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Fri, 23 Apr 2021 11:18:51 +0300 Subject: [PATCH 27/48] fix for block transforms --- packages/block-library/src/heading/edit.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index 0308d1d07e8aa..d78cd5544a83f 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -56,8 +56,9 @@ function HeadingEdit( { // Handle content changes. useEffect( () => { if ( - attributes.isAnchorAutoGenerated && - generatedAnchor !== attributes.anchor + undefined === attributes.anchor || + ( attributes.isAnchorAutoGenerated && + generatedAnchor !== attributes.anchor ) ) { setAttributes( { anchor: generatedAnchor } ); } From 370409e8d42860ad2ebd06fe1e4936f9c262a6cb Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Mon, 26 Apr 2021 10:14:42 +0300 Subject: [PATCH 28/48] Improve generated slugs --- .../src/heading/autogenerate-anchors.js | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index fda20fe44dcee..63a4914f0f783 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -6,7 +6,6 @@ import { deburr, trim } from 'lodash'; /** * WordPress dependencies */ -import { cleanForSlug } from '@wordpress/url'; import { useSelect } from '@wordpress/data'; import { store as blockEditorStore } from '@wordpress/block-editor'; @@ -73,23 +72,13 @@ const getAllHeadingAnchors = ( blockList, excludeId ) => { * @return {string} Returns the slug. */ const getSlug = ( content ) => { - content = getTextWithoutMarkup( content ); - // Get the slug. - let slug = cleanForSlug( content ); - - // If slug is empty, then there is no content, or content is using non-latin characters. - // Try non-latin first. - if ( '' === slug ) { - slug = trim( - deburr( content ) - .replace( /[\s\./]+/g, '-' ) - .toLowerCase(), - '-' - ); - } - - return slug; + return trim( + deburr( getTextWithoutMarkup( content ) ) + .replace( /[^\p{L}\p{N}]+/gu, '-' ) + .toLowerCase(), + '-' + ); }; /** From 951480cdc6418fcea90c2595b931ad4442cf6b49 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Mon, 26 Apr 2021 11:46:26 +0300 Subject: [PATCH 29/48] bring back the prefix --- .../src/heading/autogenerate-anchors.js | 6 +++-- packages/block-library/src/heading/edit.js | 22 +------------------ 2 files changed, 5 insertions(+), 23 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index 63a4914f0f783..cdf408d2d9b2a 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -98,7 +98,7 @@ const generateAnchor = ( anchor, content, allHeadingAnchors ) => { return null; } - const baseAnchor = slug; + const baseAnchor = `wp-${ slug }`; anchor = baseAnchor; let i = 0; @@ -128,5 +128,7 @@ export default function useGeneratedAnchor( clientId, anchor, content ) { }, [ clientId ] ); - return generateAnchor( anchor, content, allHeadingAnchors ); + return ! anchor || anchor.startsWith( 'wp-' ) + ? generateAnchor( anchor, content, allHeadingAnchors ) + : anchor; } diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index d78cd5544a83f..bc86792acb336 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -44,33 +44,13 @@ function HeadingEdit( { content ); - // Set the isAnchorAutoGenerated attribute. - // Please note that this attribute does not get saved. - if ( - generatedAnchor === attributes.anchor || - ( ! attributes.anchor && ! generatedAnchor ) - ) { - setAttributes( { isAnchorAutoGenerated: true } ); - } - // Handle content changes. useEffect( () => { - if ( - undefined === attributes.anchor || - ( attributes.isAnchorAutoGenerated && - generatedAnchor !== attributes.anchor ) - ) { + if ( generatedAnchor !== attributes.anchor ) { setAttributes( { anchor: generatedAnchor } ); } }, [ generatedAnchor ] ); - // Handle anchor changes. - useEffect( () => { - setAttributes( { - isAnchorAutoGenerated: generatedAnchor === attributes.anchor, - } ); - }, [ attributes.anchor ] ); - return ( <> From 9953f7065299a7cce81829eda739310ee7422ea9 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Fri, 23 Apr 2021 08:47:23 +0300 Subject: [PATCH 30/48] Revert "update e2e" This reverts commit 792cfadff2d73fff8953fd7242ca11275ef8f643. --- .../blocks/__snapshots__/heading.test.js.snap | 16 +++++++-------- .../blocks/__snapshots__/quote.test.js.snap | 20 +++++++++---------- .../__snapshots__/block-grouping.test.js.snap | 8 ++++---- .../inserting-blocks.test.js.snap | 4 ++-- .../specs/widgets/adding-widgets.test.js | 2 +- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/packages/e2e-tests/specs/editor/blocks/__snapshots__/heading.test.js.snap b/packages/e2e-tests/specs/editor/blocks/__snapshots__/heading.test.js.snap index 290019372feeb..aae12c11ca0d9 100644 --- a/packages/e2e-tests/specs/editor/blocks/__snapshots__/heading.test.js.snap +++ b/packages/e2e-tests/specs/editor/blocks/__snapshots__/heading.test.js.snap @@ -2,25 +2,25 @@ exports[`Heading can be created by prefixing existing content with number signs and a space 1`] = ` " -

4

+

4

" `; exports[`Heading can be created by prefixing number sign and a space 1`] = ` " -

3

+

3

" `; exports[`Heading should correctly apply custom colors 1`] = ` " -

Heading

+

Heading

" `; exports[`Heading should correctly apply named colors 1`] = ` " -

Heading

+

Heading

" `; @@ -30,13 +30,13 @@ exports[`Heading should create a paragraph block above when pressing enter at th -

a

+

a

" `; exports[`Heading should create a paragraph block below when pressing enter at the end 1`] = ` " -

a

+

a

@@ -46,12 +46,12 @@ exports[`Heading should create a paragraph block below when pressing enter at th exports[`Heading should not work with the list input rule 1`] = ` " -

1. H

+

1. H

" `; exports[`Heading should work with the format input rules 1`] = ` " -

code

+

code

" `; diff --git a/packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap b/packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap index 6345efae5ed72..5b182f49bb5c4 100644 --- a/packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap +++ b/packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap @@ -40,7 +40,7 @@ exports[`Quote can be converted to paragraphs and renders only one paragraph for exports[`Quote can be created by converting a heading 1`] = ` " -

test

+

test

" `; @@ -144,7 +144,7 @@ exports[`Quote can be split in the middle and merged back 4`] = ` exports[`Quote is transformed to a heading and a quote if the quote contains a citation 1`] = ` " -

one

+

one

@@ -154,7 +154,7 @@ exports[`Quote is transformed to a heading and a quote if the quote contains a c exports[`Quote is transformed to a heading and a quote if the quote contains multiple paragraphs 1`] = ` " -

one

+

one

@@ -164,7 +164,7 @@ exports[`Quote is transformed to a heading and a quote if the quote contains mul exports[`Quote is transformed to a heading if the quote just contains one paragraph 1`] = ` " -

one

+

one

" `; @@ -176,7 +176,7 @@ exports[`Quote is transformed to an empty heading if the quote is empty 1`] = ` exports[`Quote the resuling quote after transforming to a heading can be transformed again 1`] = ` " -

one

+

one

@@ -186,11 +186,11 @@ exports[`Quote the resuling quote after transforming to a heading can be transfo exports[`Quote the resuling quote after transforming to a heading can be transformed again 2`] = ` " -

one

+

one

-

two

+

two

@@ -200,14 +200,14 @@ exports[`Quote the resuling quote after transforming to a heading can be transfo exports[`Quote the resuling quote after transforming to a heading can be transformed again 3`] = ` " -

one

+

one

-

two

+

two

-

cite

+

cite

" `; diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/block-grouping.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/block-grouping.test.js.snap index aeaafed4ea8a7..62b4bebfc9914 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/block-grouping.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/block-grouping.test.js.snap @@ -3,7 +3,7 @@ exports[`Block Grouping Group creation creates a group from multiple blocks of different types via block transforms 1`] = ` "
-

Group Heading

+

Group Heading

@@ -51,7 +51,7 @@ exports[`Block Grouping Group creation creates a group from multiple blocks of t exports[`Block Grouping Group creation groups and ungroups multiple blocks of different types via options toolbar 1`] = ` "
-

Group Heading

+

Group Heading

@@ -66,7 +66,7 @@ exports[`Block Grouping Group creation groups and ungroups multiple blocks of di exports[`Block Grouping Group creation groups and ungroups multiple blocks of different types via options toolbar 2`] = ` " -

Group Heading

+

Group Heading

@@ -81,7 +81,7 @@ exports[`Block Grouping Group creation groups and ungroups multiple blocks of di exports[`Block Grouping Preserving selected blocks attributes preserves width alignment settings of selected blocks 1`] = ` "
-

Group Heading

+

Group Heading

diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/inserting-blocks.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/inserting-blocks.test.js.snap index b97b80ddf59ae..403eb377d3861 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/inserting-blocks.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/inserting-blocks.test.js.snap @@ -71,7 +71,7 @@ exports[`Inserting blocks inserts a block in proper place after having clicked \ -

Heading

+

Heading

@@ -93,7 +93,7 @@ exports[`Inserting blocks inserts a block in proper place after having clicked \ -

Heading

+

Heading

diff --git a/packages/e2e-tests/specs/widgets/adding-widgets.test.js b/packages/e2e-tests/specs/widgets/adding-widgets.test.js index 2fa6a61ab951f..b19097cff18f5 100644 --- a/packages/e2e-tests/specs/widgets/adding-widgets.test.js +++ b/packages/e2e-tests/specs/widgets/adding-widgets.test.js @@ -347,7 +347,7 @@ describe( 'Widgets screen', () => {

First Paragraph

-

My Heading

+

My Heading

Second Paragraph

From 694180e2a52b780454b97a04444198db05d6e97b Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Mon, 26 Apr 2021 12:59:01 +0300 Subject: [PATCH 31/48] remove wp- prefix if not autogenerated anchor --- packages/block-library/src/heading/edit.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index bc86792acb336..d88580465feda 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -44,6 +44,16 @@ function HeadingEdit( { content ); + // If the anchor is not autogenerated, remove the auto-generated `wp-` prefix. + if ( + generatedAnchor !== attributes.anchor && + attributes.anchor.startsWith( 'wp-' ) + ) { + setAttributes( { + anchor: attributes.anchor.replace( 'wp-', '' ), + } ); + } + // Handle content changes. useEffect( () => { if ( generatedAnchor !== attributes.anchor ) { From 34d17212138e594d440d8903f3db58aa1be843f1 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Mon, 26 Apr 2021 13:06:21 +0300 Subject: [PATCH 32/48] fix for empty/null anchors --- packages/block-library/src/heading/edit.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index d88580465feda..4992e9bbaf8fc 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -46,6 +46,7 @@ function HeadingEdit( { // If the anchor is not autogenerated, remove the auto-generated `wp-` prefix. if ( + attributes.anchor && generatedAnchor !== attributes.anchor && attributes.anchor.startsWith( 'wp-' ) ) { From 50141d797e08e279a12f22d641e2a9bbd1411a1b Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Mon, 26 Apr 2021 13:26:11 +0300 Subject: [PATCH 33/48] use useEffect properly --- packages/block-library/src/heading/edit.js | 29 ++++++++++++---------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index 4992e9bbaf8fc..daa1b48de36cd 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -44,24 +44,27 @@ function HeadingEdit( { content ); - // If the anchor is not autogenerated, remove the auto-generated `wp-` prefix. - if ( - attributes.anchor && - generatedAnchor !== attributes.anchor && - attributes.anchor.startsWith( 'wp-' ) - ) { - setAttributes( { - anchor: attributes.anchor.replace( 'wp-', '' ), - } ); - } - - // Handle content changes. + // Update anchor when the content changes. useEffect( () => { - if ( generatedAnchor !== attributes.anchor ) { + if ( generatedAnchor && generatedAnchor.startsWith( 'wp-' ) ) { setAttributes( { anchor: generatedAnchor } ); } }, [ generatedAnchor ] ); + // Make sure manually-edited anchors don't have the `wp-` prefix. + // `wp-` marks an anchor as auto-generated. + useEffect( () => { + if ( + attributes.anchor && + generatedAnchor !== attributes.anchor && + attributes.anchor.startsWith( 'wp-' ) + ) { + setAttributes( { + anchor: attributes.anchor.replace( 'wp-', '' ), + } ); + } + }, [ attributes.anchor ] ); + return ( <> From f1ad7c2cc61795fb632f53c9cdbbd6686d62e7d9 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 5 May 2021 10:50:03 +0300 Subject: [PATCH 34/48] Refactor to not use a prefix. --- .../src/heading/autogenerate-anchors.js | 36 ++++++++++++------- packages/block-library/src/heading/edit.js | 21 +++-------- 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index cdf408d2d9b2a..1da9e9bd4635f 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -84,13 +84,12 @@ const getSlug = ( content ) => { /** * Generate the anchor for a heading. * - * @param {string} anchor The heading anchor. * @param {string} content The block content. * @param {string[]} allHeadingAnchors An array containing all headings anchors. * * @return {string|null} Return the heading anchor. */ -const generateAnchor = ( anchor, content, allHeadingAnchors ) => { +const generateAnchor = ( content, allHeadingAnchors ) => { const slug = getSlug( content ); // If slug is empty, then return null. // Returning null instead of an empty string allows us to check again when the content changes. @@ -98,14 +97,13 @@ const generateAnchor = ( anchor, content, allHeadingAnchors ) => { return null; } - const baseAnchor = `wp-${ slug }`; - anchor = baseAnchor; + let anchor = slug; let i = 0; // If the anchor already exists in another heading, append -i. - while ( allHeadingAnchors.includes( anchor ) ) { + while ( allHeadingAnchors.includes( slug ) ) { i += 1; - anchor = baseAnchor + '-' + i; + anchor = slug + '-' + i; } return anchor; @@ -114,13 +112,19 @@ const generateAnchor = ( anchor, content, allHeadingAnchors ) => { /** * Updates the anchor if required. * - * @param {string} clientId The block's client-ID. - * @param {string} anchor The heading anchor. - * @param {string} content The block content. + * @param {string} clientId The block's client-ID. + * @param {string} anchor The heading anchor. + * @param {string} prevContent The previous value of the content. + * @param {string} content The block content. * * @return {string} The anchor. */ -export default function useGeneratedAnchor( clientId, anchor, content ) { +export default function useGeneratedAnchor( + clientId, + anchor, + prevContent, + content +) { const allHeadingAnchors = useSelect( ( select ) => { const allBlocks = select( blockEditorStore ).getBlocks(); @@ -128,7 +132,13 @@ export default function useGeneratedAnchor( clientId, anchor, content ) { }, [ clientId ] ); - return ! anchor || anchor.startsWith( 'wp-' ) - ? generateAnchor( anchor, content, allHeadingAnchors ) - : anchor; + + if ( + ! anchor || + '' === content || + generateAnchor( prevContent, allHeadingAnchors ) === anchor + ) { + return generateAnchor( content, allHeadingAnchors ); + } + return anchor; } diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index daa1b48de36cd..9561e8e542ddd 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -15,6 +15,7 @@ import { useBlockProps, } from '@wordpress/block-editor'; import { useEffect } from '@wordpress/element'; +import { usePrevious } from '@wordpress/compose'; /** * Internal dependencies @@ -38,32 +39,20 @@ function HeadingEdit( { } ), style: mergedStyle, } ); + const prevContent = usePrevious( content ); const generatedAnchor = useGeneratedAnchor( clientId, attributes.anchor, + prevContent, content ); // Update anchor when the content changes. useEffect( () => { - if ( generatedAnchor && generatedAnchor.startsWith( 'wp-' ) ) { + if ( generatedAnchor !== attributes.anchor ) { setAttributes( { anchor: generatedAnchor } ); } - }, [ generatedAnchor ] ); - - // Make sure manually-edited anchors don't have the `wp-` prefix. - // `wp-` marks an anchor as auto-generated. - useEffect( () => { - if ( - attributes.anchor && - generatedAnchor !== attributes.anchor && - attributes.anchor.startsWith( 'wp-' ) - ) { - setAttributes( { - anchor: attributes.anchor.replace( 'wp-', '' ), - } ); - } - }, [ attributes.anchor ] ); + }, [ attributes.anchor, generatedAnchor ] ); return ( <> From c61d9a75a24461f7d8ce20b65919eb4942b2fe1c Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 5 May 2021 11:03:01 +0300 Subject: [PATCH 35/48] Maybe not use setAttributes? :thinking: --- packages/block-library/src/heading/edit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index 9561e8e542ddd..89d473fb3d305 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -50,7 +50,7 @@ function HeadingEdit( { // Update anchor when the content changes. useEffect( () => { if ( generatedAnchor !== attributes.anchor ) { - setAttributes( { anchor: generatedAnchor } ); + attributes.anchor = generatedAnchor; } }, [ attributes.anchor, generatedAnchor ] ); From 7d8c3eaf01413ff0a3cf3105c47704fa9987180c Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 5 May 2021 11:15:22 +0300 Subject: [PATCH 36/48] no need for this condition --- packages/block-library/src/heading/edit.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index 89d473fb3d305..9c47ee22b175d 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -49,9 +49,7 @@ function HeadingEdit( { // Update anchor when the content changes. useEffect( () => { - if ( generatedAnchor !== attributes.anchor ) { - attributes.anchor = generatedAnchor; - } + attributes.anchor = generatedAnchor; }, [ attributes.anchor, generatedAnchor ] ); return ( From 00006fe720fc8171a5cc2245ae13458cc63fad9b Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 5 May 2021 11:58:35 +0300 Subject: [PATCH 37/48] This was too ugly to stay for more than 10 minutes --- packages/block-library/src/heading/edit.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index 9c47ee22b175d..9561e8e542ddd 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -49,7 +49,9 @@ function HeadingEdit( { // Update anchor when the content changes. useEffect( () => { - attributes.anchor = generatedAnchor; + if ( generatedAnchor !== attributes.anchor ) { + setAttributes( { anchor: generatedAnchor } ); + } }, [ attributes.anchor, generatedAnchor ] ); return ( From 7b8b78395ed33e4801da58ed2ca994d283756c3f Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 5 May 2021 12:05:30 +0300 Subject: [PATCH 38/48] remove prefix from tests --- .../blocks/__snapshots__/heading.test.js.snap | 16 +++++++-------- .../blocks/__snapshots__/quote.test.js.snap | 20 +++++++++---------- .../__snapshots__/block-grouping.test.js.snap | 8 ++++---- .../inserting-blocks.test.js.snap | 4 ++-- .../specs/widgets/adding-widgets.test.js | 2 +- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/packages/e2e-tests/specs/editor/blocks/__snapshots__/heading.test.js.snap b/packages/e2e-tests/specs/editor/blocks/__snapshots__/heading.test.js.snap index aae12c11ca0d9..290019372feeb 100644 --- a/packages/e2e-tests/specs/editor/blocks/__snapshots__/heading.test.js.snap +++ b/packages/e2e-tests/specs/editor/blocks/__snapshots__/heading.test.js.snap @@ -2,25 +2,25 @@ exports[`Heading can be created by prefixing existing content with number signs and a space 1`] = ` " -

4

+

4

" `; exports[`Heading can be created by prefixing number sign and a space 1`] = ` " -

3

+

3

" `; exports[`Heading should correctly apply custom colors 1`] = ` " -

Heading

+

Heading

" `; exports[`Heading should correctly apply named colors 1`] = ` " -

Heading

+

Heading

" `; @@ -30,13 +30,13 @@ exports[`Heading should create a paragraph block above when pressing enter at th -

a

+

a

" `; exports[`Heading should create a paragraph block below when pressing enter at the end 1`] = ` " -

a

+

a

@@ -46,12 +46,12 @@ exports[`Heading should create a paragraph block below when pressing enter at th exports[`Heading should not work with the list input rule 1`] = ` " -

1. H

+

1. H

" `; exports[`Heading should work with the format input rules 1`] = ` " -

code

+

code

" `; diff --git a/packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap b/packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap index 5b182f49bb5c4..6345efae5ed72 100644 --- a/packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap +++ b/packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap @@ -40,7 +40,7 @@ exports[`Quote can be converted to paragraphs and renders only one paragraph for exports[`Quote can be created by converting a heading 1`] = ` " -

test

+

test

" `; @@ -144,7 +144,7 @@ exports[`Quote can be split in the middle and merged back 4`] = ` exports[`Quote is transformed to a heading and a quote if the quote contains a citation 1`] = ` " -

one

+

one

@@ -154,7 +154,7 @@ exports[`Quote is transformed to a heading and a quote if the quote contains a c exports[`Quote is transformed to a heading and a quote if the quote contains multiple paragraphs 1`] = ` " -

one

+

one

@@ -164,7 +164,7 @@ exports[`Quote is transformed to a heading and a quote if the quote contains mul exports[`Quote is transformed to a heading if the quote just contains one paragraph 1`] = ` " -

one

+

one

" `; @@ -176,7 +176,7 @@ exports[`Quote is transformed to an empty heading if the quote is empty 1`] = ` exports[`Quote the resuling quote after transforming to a heading can be transformed again 1`] = ` " -

one

+

one

@@ -186,11 +186,11 @@ exports[`Quote the resuling quote after transforming to a heading can be transfo exports[`Quote the resuling quote after transforming to a heading can be transformed again 2`] = ` " -

one

+

one

-

two

+

two

@@ -200,14 +200,14 @@ exports[`Quote the resuling quote after transforming to a heading can be transfo exports[`Quote the resuling quote after transforming to a heading can be transformed again 3`] = ` " -

one

+

one

-

two

+

two

-

cite

+

cite

" `; diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/block-grouping.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/block-grouping.test.js.snap index 62b4bebfc9914..aeaafed4ea8a7 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/block-grouping.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/block-grouping.test.js.snap @@ -3,7 +3,7 @@ exports[`Block Grouping Group creation creates a group from multiple blocks of different types via block transforms 1`] = ` "
-

Group Heading

+

Group Heading

@@ -51,7 +51,7 @@ exports[`Block Grouping Group creation creates a group from multiple blocks of t exports[`Block Grouping Group creation groups and ungroups multiple blocks of different types via options toolbar 1`] = ` "
-

Group Heading

+

Group Heading

@@ -66,7 +66,7 @@ exports[`Block Grouping Group creation groups and ungroups multiple blocks of di exports[`Block Grouping Group creation groups and ungroups multiple blocks of different types via options toolbar 2`] = ` " -

Group Heading

+

Group Heading

@@ -81,7 +81,7 @@ exports[`Block Grouping Group creation groups and ungroups multiple blocks of di exports[`Block Grouping Preserving selected blocks attributes preserves width alignment settings of selected blocks 1`] = ` "
-

Group Heading

+

Group Heading

diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/inserting-blocks.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/inserting-blocks.test.js.snap index 403eb377d3861..b97b80ddf59ae 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/inserting-blocks.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/inserting-blocks.test.js.snap @@ -71,7 +71,7 @@ exports[`Inserting blocks inserts a block in proper place after having clicked \ -

Heading

+

Heading

@@ -93,7 +93,7 @@ exports[`Inserting blocks inserts a block in proper place after having clicked \ -

Heading

+

Heading

diff --git a/packages/e2e-tests/specs/widgets/adding-widgets.test.js b/packages/e2e-tests/specs/widgets/adding-widgets.test.js index b19097cff18f5..2fa6a61ab951f 100644 --- a/packages/e2e-tests/specs/widgets/adding-widgets.test.js +++ b/packages/e2e-tests/specs/widgets/adding-widgets.test.js @@ -347,7 +347,7 @@ describe( 'Widgets screen', () => {

First Paragraph

-

My Heading

+

My Heading

Second Paragraph

From 516c4001a69c5544850327517db9d74854e737c6 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 5 May 2021 13:35:48 +0300 Subject: [PATCH 39/48] alternative to useEffect --- packages/block-library/src/heading/edit.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index 9561e8e542ddd..ccf609a56756e 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -14,7 +14,6 @@ import { RichText, useBlockProps, } from '@wordpress/block-editor'; -import { useEffect } from '@wordpress/element'; import { usePrevious } from '@wordpress/compose'; /** @@ -47,13 +46,6 @@ function HeadingEdit( { content ); - // Update anchor when the content changes. - useEffect( () => { - if ( generatedAnchor !== attributes.anchor ) { - setAttributes( { anchor: generatedAnchor } ); - } - }, [ attributes.anchor, generatedAnchor ] ); - return ( <> @@ -74,7 +66,12 @@ function HeadingEdit( { identifier="content" tagName={ tagName } value={ content } - onChange={ ( value ) => setAttributes( { content: value } ) } + onChange={ ( value ) => + setAttributes( { + content: value, + anchor: generatedAnchor, + } ) + } onMerge={ mergeBlocks } onSplit={ ( value, isOriginal ) => { let block; From 3ec7e1e4717e73b6e474e91e517cae6b7e49a57d Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 5 May 2021 13:35:48 +0300 Subject: [PATCH 40/48] Revert "alternative to useEffect" This reverts commit 516c4001a69c5544850327517db9d74854e737c6. --- packages/block-library/src/heading/edit.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index ccf609a56756e..9561e8e542ddd 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -14,6 +14,7 @@ import { RichText, useBlockProps, } from '@wordpress/block-editor'; +import { useEffect } from '@wordpress/element'; import { usePrevious } from '@wordpress/compose'; /** @@ -46,6 +47,13 @@ function HeadingEdit( { content ); + // Update anchor when the content changes. + useEffect( () => { + if ( generatedAnchor !== attributes.anchor ) { + setAttributes( { anchor: generatedAnchor } ); + } + }, [ attributes.anchor, generatedAnchor ] ); + return ( <> @@ -66,12 +74,7 @@ function HeadingEdit( { identifier="content" tagName={ tagName } value={ content } - onChange={ ( value ) => - setAttributes( { - content: value, - anchor: generatedAnchor, - } ) - } + onChange={ ( value ) => setAttributes( { content: value } ) } onMerge={ mergeBlocks } onSplit={ ( value, isOriginal ) => { let block; From 171f2289f12441aa1c7a9a7cd49f2808b35f0366 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 5 May 2021 14:26:34 +0300 Subject: [PATCH 41/48] only watch the content --- packages/block-library/src/heading/edit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index 9561e8e542ddd..a9fe5025810f1 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -52,7 +52,7 @@ function HeadingEdit( { if ( generatedAnchor !== attributes.anchor ) { setAttributes( { anchor: generatedAnchor } ); } - }, [ attributes.anchor, generatedAnchor ] ); + }, [ content ] ); return ( <> From 577feb64544621deda8f85fa8a643f3bd6be5786 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 5 May 2021 14:27:45 +0300 Subject: [PATCH 42/48] mark anchor changes as not persistent --- packages/block-library/src/heading/edit.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index a9fe5025810f1..3ff95fcb164a5 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -13,9 +13,11 @@ import { BlockControls, RichText, useBlockProps, + store as blockEditorStore, } from '@wordpress/block-editor'; import { useEffect } from '@wordpress/element'; import { usePrevious } from '@wordpress/compose'; +import { useDispatch } from '@wordpress/data'; /** * Internal dependencies @@ -46,10 +48,14 @@ function HeadingEdit( { prevContent, content ); + const { __unstableMarkNextChangeAsNotPersistent } = useDispatch( + blockEditorStore + ); // Update anchor when the content changes. useEffect( () => { if ( generatedAnchor !== attributes.anchor ) { + __unstableMarkNextChangeAsNotPersistent(); setAttributes( { anchor: generatedAnchor } ); } }, [ content ] ); From 5706771c28e29c5f4c5263cb6aed6352b2e1a555 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 5 May 2021 15:42:55 +0300 Subject: [PATCH 43/48] yet another refactor --- .../src/heading/autogenerate-anchors.js | 26 +++-------- packages/block-library/src/heading/edit.js | 44 +++++++++---------- 2 files changed, 25 insertions(+), 45 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index 1da9e9bd4635f..18352420aa1f2 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -89,7 +89,7 @@ const getSlug = ( content ) => { * * @return {string|null} Return the heading anchor. */ -const generateAnchor = ( content, allHeadingAnchors ) => { +export const generateAnchor = ( content, allHeadingAnchors ) => { const slug = getSlug( content ); // If slug is empty, then return null. // Returning null instead of an empty string allows us to check again when the content changes. @@ -112,19 +112,11 @@ const generateAnchor = ( content, allHeadingAnchors ) => { /** * Updates the anchor if required. * - * @param {string} clientId The block's client-ID. - * @param {string} anchor The heading anchor. - * @param {string} prevContent The previous value of the content. - * @param {string} content The block content. + * @param {string} clientId The block's client-ID. * * @return {string} The anchor. */ -export default function useGeneratedAnchor( - clientId, - anchor, - prevContent, - content -) { +export const Anchors = ( clientId ) => { const allHeadingAnchors = useSelect( ( select ) => { const allBlocks = select( blockEditorStore ).getBlocks(); @@ -132,13 +124,5 @@ export default function useGeneratedAnchor( }, [ clientId ] ); - - if ( - ! anchor || - '' === content || - generateAnchor( prevContent, allHeadingAnchors ) === anchor - ) { - return generateAnchor( content, allHeadingAnchors ); - } - return anchor; -} + return allHeadingAnchors; +}; diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index 3ff95fcb164a5..e642ea651b206 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -13,17 +13,13 @@ import { BlockControls, RichText, useBlockProps, - store as blockEditorStore, } from '@wordpress/block-editor'; -import { useEffect } from '@wordpress/element'; -import { usePrevious } from '@wordpress/compose'; -import { useDispatch } from '@wordpress/data'; /** * Internal dependencies */ import HeadingLevelDropdown from './heading-level-dropdown'; -import useGeneratedAnchor from './autogenerate-anchors'; +import { Anchors, generateAnchor } from './autogenerate-anchors'; function HeadingEdit( { attributes, @@ -41,24 +37,7 @@ function HeadingEdit( { } ), style: mergedStyle, } ); - const prevContent = usePrevious( content ); - const generatedAnchor = useGeneratedAnchor( - clientId, - attributes.anchor, - prevContent, - content - ); - const { __unstableMarkNextChangeAsNotPersistent } = useDispatch( - blockEditorStore - ); - - // Update anchor when the content changes. - useEffect( () => { - if ( generatedAnchor !== attributes.anchor ) { - __unstableMarkNextChangeAsNotPersistent(); - setAttributes( { anchor: generatedAnchor } ); - } - }, [ content ] ); + const allHeadingAnchors = Anchors( clientId ); return ( <> @@ -80,7 +59,24 @@ function HeadingEdit( { identifier="content" tagName={ tagName } value={ content } - onChange={ ( value ) => setAttributes( { content: value } ) } + onChange={ ( value ) => { + const newAttrs = { + content: value, + }; + if ( + ! attributes.anchor || + ! value || + generateAnchor( content, allHeadingAnchors ) === + attributes.anchor + ) { + newAttrs.anchor = generateAnchor( + value, + allHeadingAnchors + ); + } + + setAttributes( newAttrs ); + } } onMerge={ mergeBlocks } onSplit={ ( value, isOriginal ) => { let block; From 60114f1199917fffc0c03a14ed466854186d674d Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 5 May 2021 16:30:24 +0300 Subject: [PATCH 44/48] fix for legacy headers & conversions --- packages/block-library/src/heading/edit.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index e642ea651b206..4b2ec5e104f80 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -39,6 +39,14 @@ function HeadingEdit( { } ); const allHeadingAnchors = Anchors( clientId ); + // Initially set anchor for headings that have content but no anchor set. + // This is used when transforming a block to heading, or for legacy anchors. + if ( ! attributes.anchor && content ) { + setAttributes( { + anchor: generateAnchor( content, allHeadingAnchors ), + } ); + } + return ( <> From 0dbefeb4b16968a6403792819a734f97ec59f157 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 5 May 2021 16:42:06 +0300 Subject: [PATCH 45/48] Typo was causing a crash --- packages/block-library/src/heading/autogenerate-anchors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index 18352420aa1f2..4f2eb0c268f47 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -101,7 +101,7 @@ export const generateAnchor = ( content, allHeadingAnchors ) => { let i = 0; // If the anchor already exists in another heading, append -i. - while ( allHeadingAnchors.includes( slug ) ) { + while ( allHeadingAnchors.includes( anchor ) ) { i += 1; anchor = slug + '-' + i; } From 2a021a63833b26caf7594d657f342e5896c18290 Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Thu, 6 May 2021 12:20:44 +0300 Subject: [PATCH 46/48] use useEffect for side effects --- .../src/heading/autogenerate-anchors.js | 9 ++-- packages/block-library/src/heading/edit.js | 50 +++++++++---------- 2 files changed, 28 insertions(+), 31 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index 4f2eb0c268f47..1b5a53597b1ad 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -110,19 +110,18 @@ export const generateAnchor = ( content, allHeadingAnchors ) => { }; /** - * Updates the anchor if required. + * Retrieves and returns all heading anchors. * * @param {string} clientId The block's client-ID. * - * @return {string} The anchor. + * @return {string[]} The array of heading anchors. */ -export const Anchors = ( clientId ) => { - const allHeadingAnchors = useSelect( +export const useAllHeadingAnchors = ( clientId ) => { + return useSelect( ( select ) => { const allBlocks = select( blockEditorStore ).getBlocks(); return getAllHeadingAnchors( allBlocks, clientId ); }, [ clientId ] ); - return allHeadingAnchors; }; diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index 4b2ec5e104f80..d41c72005de9d 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -7,6 +7,7 @@ import classnames from 'classnames'; * WordPress dependencies */ import { __ } from '@wordpress/i18n'; +import { useEffect } from '@wordpress/element'; import { createBlock } from '@wordpress/blocks'; import { AlignmentControl, @@ -19,7 +20,7 @@ import { * Internal dependencies */ import HeadingLevelDropdown from './heading-level-dropdown'; -import { Anchors, generateAnchor } from './autogenerate-anchors'; +import { useAllHeadingAnchors, generateAnchor } from './autogenerate-anchors'; function HeadingEdit( { attributes, @@ -29,7 +30,7 @@ function HeadingEdit( { mergedStyle, clientId, } ) { - const { textAlign, content, level, placeholder } = attributes; + const { textAlign, content, level, placeholder, anchor } = attributes; const tagName = 'h' + level; const blockProps = useBlockProps( { className: classnames( { @@ -37,15 +38,29 @@ function HeadingEdit( { } ), style: mergedStyle, } ); - const allHeadingAnchors = Anchors( clientId ); + const allHeadingAnchors = useAllHeadingAnchors( clientId ); // Initially set anchor for headings that have content but no anchor set. // This is used when transforming a block to heading, or for legacy anchors. - if ( ! attributes.anchor && content ) { - setAttributes( { - anchor: generateAnchor( content, allHeadingAnchors ), - } ); - } + useEffect( () => { + if ( ! anchor && content ) { + setAttributes( { + anchor: generateAnchor( content, allHeadingAnchors ), + } ); + } + }, [ allHeadingAnchors ] ); + + const onContentChange = ( value ) => { + const newAttrs = { content: value }; + if ( + ! anchor || + ! value || + generateAnchor( content, allHeadingAnchors ) === anchor + ) { + newAttrs.anchor = generateAnchor( value, allHeadingAnchors ); + } + setAttributes( newAttrs ); + }; return ( <> @@ -67,24 +82,7 @@ function HeadingEdit( { identifier="content" tagName={ tagName } value={ content } - onChange={ ( value ) => { - const newAttrs = { - content: value, - }; - if ( - ! attributes.anchor || - ! value || - generateAnchor( content, allHeadingAnchors ) === - attributes.anchor - ) { - newAttrs.anchor = generateAnchor( - value, - allHeadingAnchors - ); - } - - setAttributes( newAttrs ); - } } + onChange={ onContentChange } onMerge={ mergeBlocks } onSplit={ ( value, isOriginal ) => { let block; From 5bb9302bb8bc3f0f87ad793c25bfea38fe2e6ae2 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Tue, 20 Jul 2021 13:22:07 +0300 Subject: [PATCH 47/48] Simplify & improve performance --- .../src/heading/autogenerate-anchors.js | 72 ++----------------- packages/block-library/src/heading/edit.js | 18 +++-- 2 files changed, 17 insertions(+), 73 deletions(-) diff --git a/packages/block-library/src/heading/autogenerate-anchors.js b/packages/block-library/src/heading/autogenerate-anchors.js index 1b5a53597b1ad..ba8150e6e7e69 100644 --- a/packages/block-library/src/heading/autogenerate-anchors.js +++ b/packages/block-library/src/heading/autogenerate-anchors.js @@ -3,30 +3,6 @@ */ import { deburr, trim } from 'lodash'; -/** - * WordPress dependencies - */ -import { useSelect } from '@wordpress/data'; -import { store as blockEditorStore } from '@wordpress/block-editor'; - -/** - * Runs a callback over all blocks, including nested blocks. - * - * @param {Object[]} blocks The blocks. - * @param {Function} callback The callback. - * - * @return {void} - */ -const recurseOverBlocks = ( blocks, callback ) => { - for ( const block of blocks ) { - // eslint-disable-next-line callback-return - callback( block ); - if ( block.innerBlocks ) { - recurseOverBlocks( block.innerBlocks, callback ); - } - } -}; - /** * Returns the text without markup. * @@ -40,30 +16,6 @@ const getTextWithoutMarkup = ( text ) => { return dummyElement.innerText; }; -/** - * Get all heading anchors. - * - * @param {Object} blockList An object containing all blocks. - * @param {string} excludeId A block ID we want to exclude. - * - * @return {string[]} Return an array of anchors. - */ -const getAllHeadingAnchors = ( blockList, excludeId ) => { - const anchors = []; - - recurseOverBlocks( blockList, ( block ) => { - if ( - block.name === 'core/heading' && - ( ! excludeId || block.clientId !== excludeId ) && - block.attributes.anchor - ) { - anchors.push( block.attributes.anchor ); - } - } ); - - return anchors; -}; - /** * Get the slug from the content. * @@ -84,12 +36,13 @@ const getSlug = ( content ) => { /** * Generate the anchor for a heading. * + * @param {string} clientId The block ID. * @param {string} content The block content. * @param {string[]} allHeadingAnchors An array containing all headings anchors. * * @return {string|null} Return the heading anchor. */ -export const generateAnchor = ( content, allHeadingAnchors ) => { +export const generateAnchor = ( clientId, content, allHeadingAnchors ) => { const slug = getSlug( content ); // If slug is empty, then return null. // Returning null instead of an empty string allows us to check again when the content changes. @@ -97,31 +50,16 @@ export const generateAnchor = ( content, allHeadingAnchors ) => { return null; } + delete allHeadingAnchors[ clientId ]; + let anchor = slug; let i = 0; // If the anchor already exists in another heading, append -i. - while ( allHeadingAnchors.includes( anchor ) ) { + while ( Object.values( allHeadingAnchors ).includes( anchor ) ) { i += 1; anchor = slug + '-' + i; } return anchor; }; - -/** - * Retrieves and returns all heading anchors. - * - * @param {string} clientId The block's client-ID. - * - * @return {string[]} The array of heading anchors. - */ -export const useAllHeadingAnchors = ( clientId ) => { - return useSelect( - ( select ) => { - const allBlocks = select( blockEditorStore ).getBlocks(); - return getAllHeadingAnchors( allBlocks, clientId ); - }, - [ clientId ] - ); -}; diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index 0bd7a3d08c743..8f7d7e7d21f3d 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -20,7 +20,9 @@ import { * Internal dependencies */ import HeadingLevelDropdown from './heading-level-dropdown'; -import { useAllHeadingAnchors, generateAnchor } from './autogenerate-anchors'; +import { generateAnchor } from './autogenerate-anchors'; + +const allHeadingAnchors = new Set(); function HeadingEdit( { attributes, @@ -38,26 +40,30 @@ function HeadingEdit( { } ), style, } ); - const allHeadingAnchors = useAllHeadingAnchors( clientId ); // Initially set anchor for headings that have content but no anchor set. // This is used when transforming a block to heading, or for legacy anchors. useEffect( () => { if ( ! anchor && content ) { setAttributes( { - anchor: generateAnchor( content, allHeadingAnchors ), + anchor: generateAnchor( clientId, content, allHeadingAnchors ), } ); } - }, [ allHeadingAnchors ] ); + allHeadingAnchors[ clientId ] = anchor; + }, [ content, anchor ] ); const onContentChange = ( value ) => { const newAttrs = { content: value }; if ( ! anchor || ! value || - generateAnchor( content, allHeadingAnchors ) === anchor + generateAnchor( clientId, content, allHeadingAnchors ) === anchor ) { - newAttrs.anchor = generateAnchor( value, allHeadingAnchors ); + newAttrs.anchor = generateAnchor( + clientId, + value, + allHeadingAnchors + ); } setAttributes( newAttrs ); }; From 98d29dc9f85058052aed55db1a3603f8d41fa042 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Wed, 21 Jul 2021 10:12:11 +0300 Subject: [PATCH 48/48] Use an object instead of Set --- packages/block-library/src/heading/edit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/heading/edit.js b/packages/block-library/src/heading/edit.js index 8f7d7e7d21f3d..9ca81e0410a66 100644 --- a/packages/block-library/src/heading/edit.js +++ b/packages/block-library/src/heading/edit.js @@ -22,7 +22,7 @@ import { import HeadingLevelDropdown from './heading-level-dropdown'; import { generateAnchor } from './autogenerate-anchors'; -const allHeadingAnchors = new Set(); +const allHeadingAnchors = {}; function HeadingEdit( { attributes,