Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Table of Contents: Try maintaining block example attributes #65549

Merged
merged 3 commits into from
Sep 23, 2024
Merged

Table of Contents: Try maintaining block example attributes #65549

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d01d240
Table of Contents: Try maintaining block example attributes
aaronrobertshaw Sep 23, 2024
3985779
Revert "Table of Contents: Try maintaining block example attributes"
aaronrobertshaw Sep 23, 2024
1613438
Use innerBlocks to enforce example
aaronrobertshaw Sep 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/block-library/src/table-of-contents/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export default function TableOfContentsEdit( {
clientId,
setAttributes,
} ) {
useObserveHeadings( clientId );
useObserveHeadings( clientId, headings );

const blockProps = useBlockProps();
const instanceId = useInstanceId(
Expand Down
28 changes: 20 additions & 8 deletions packages/block-library/src/table-of-contents/hooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import fastDeepEqual from 'fast-deep-equal/es6';
/**
* WordPress dependencies
*/
import { useRegistry } from '@wordpress/data';
import { store as blocksStore } from '@wordpress/blocks';
import { useRegistry, useSelect } from '@wordpress/data';
import { __unstableStripHTML as stripHTML } from '@wordpress/dom';
import { useEffect } from '@wordpress/element';
import { addQueryArgs, removeQueryArgs } from '@wordpress/url';
Expand Down Expand Up @@ -144,13 +145,24 @@ function observeCallback( select, dispatch, clientId ) {
}
}

export function useObserveHeadings( clientId ) {
export function useObserveHeadings( clientId, headings ) {
const registry = useRegistry();
const blockType = useSelect(
( select ) =>
select( blocksStore ).getBlockType( 'core/table-of-contents' ),
[]
);
const isExample =
JSON.stringify( blockType.example.attributes.headings ) ===
JSON.stringify( headings );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I struggled to find a better option to avoid the useObserveHeadings hook overriding arbitrary block example attributes. More than happy to hear better approaches if you have them 🙏


useEffect( () => {
// Todo: Limit subscription to block editor store when data no longer depends on `getPermalink`.
// See: https://github.com/WordPress/gutenberg/pull/45513
return registry.subscribe( () =>
observeCallback( registry.select, registry.dispatch, clientId )
);
}, [ registry, clientId ] );
if ( ! isExample ) {
// Todo: Limit subscription to block editor store when data no longer depends on `getPermalink`.
// See: https://github.com/WordPress/gutenberg/pull/45513
return registry.subscribe( () =>
observeCallback( registry.select, registry.dispatch, clientId )
);
}
}, [ registry, clientId, isExample ] );
}
22 changes: 22 additions & 0 deletions packages/block-library/src/table-of-contents/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,28 @@ export const settings = {
icon,
edit,
save,
example: {
Copy link
Member

Choose a reason for hiding this comment

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

Could this be moved to block.json?

I think the example check above might be unnecessary if we use innerBlocks (?)

	"example": {
		"innerBlocks": [
			{
				"name": "core/heading",
				"attributes": {
					"level": 2,
					"content": "Heading"
				}
			},
			{
				"name": "core/heading",
				"attributes": {
					"level": 3,
					"content": "Subheading"
				}
			},
			{
				"name": "core/heading",
				"attributes": {
					"level": 2,
					"content": "Heading"
				}
			},
			{
				"name": "core/heading",
				"attributes": {
					"level": 3,
					"content": "Subheading"
				}
			}
		],
		"attributes": {
			"headings": [
				{
					"content": "Heading",
					"level": 2
				},
				{
					"content": "Subheading",
					"level": 3
				},
				{
					"content": "Heading",
					"level": 2
				},
				{
					"content": "Subheading",
					"level": 3
				}
			]
		}
	},
Screenshot 2024-09-23 at 11 45 29 am

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, that works, thanks 🙇

I take it that the definition of the innerBlocks and them being rendered initially for the example means there are heading blocks in the registry. That then leads the useObserveHeadings hook to find those before updating the block's headings attribute.

If I remove the attributes.headings value, you can see the block example start in the placeholder state then update to match the provided inner blocks.

I've pushed a commit using the suggested approach.

Copy link
Member

Choose a reason for hiding this comment

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

I take it that the definition of the innerBlocks and them being rendered initially for the example means there are heading blocks in the registry. That then leads the useObserveHeadings hook to find those before updating the block's headings attribute.

That sounds legit. My brain stops functioning when I see something working. 😄

attributes: {
headings: [
{
content: 'Heading',
level: 2,
},
{
content: 'Subheading',
level: 3,
},
{
content: 'Heading',
level: 2,
},
{
content: 'Subheading',
level: 3,
},
],
},
},
};

export const init = () => initBlock( { name, metadata, settings } );
Loading