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.

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

Add Table of Contents block #21040

Closed
wants to merge 39 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
e5bea61
Copy changes from pull request #15426 (https://github.com/WordPress/g…
SeanDS Mar 20, 2020
ba40205
Update packages/block-library/src/table-of-contents/utils.js
SeanDS Mar 20, 2020
ba5c43a
Update packages/block-library/src/table-of-contents/utils.js
SeanDS Mar 20, 2020
8507968
Update packages/block-library/src/table-of-contents/utils.js
SeanDS Mar 20, 2020
50df5b5
Update packages/block-library/src/table-of-contents/utils.js
SeanDS Mar 20, 2020
4c650b5
Update packages/block-library/src/table-of-contents/utils.js
SeanDS Mar 20, 2020
371a901
Merge branch 'master' into add/table-of-contents
SeanDS Mar 24, 2020
d2e929b
Merge branch 'master' into add/table-of-contents
SeanDS Mar 25, 2020
c0f2e1a
Fix variable name capitalisation
SeanDS Mar 26, 2020
4fce9a1
Rename function
SeanDS Mar 26, 2020
cb3e495
Reword description
SeanDS Mar 26, 2020
bcfc383
Change argument from array to headingsList
SeanDS Mar 26, 2020
f6aeb36
Change variable name
SeanDS Mar 26, 2020
366d7bd
Add comment
SeanDS Mar 26, 2020
0648776
Get rid of autosync (users should now convert to list if they want to…
SeanDS Mar 26, 2020
29e3c05
Add ability to transform into list; remove unused ListLevel props
SeanDS Mar 26, 2020
5cfade2
Update table-of-contents block test configuration
SeanDS Mar 26, 2020
b89368d
Simplify expression
SeanDS Mar 26, 2020
b82a9e5
Remove unused function
SeanDS Mar 26, 2020
7e4a236
Remove unused style
SeanDS Mar 26, 2020
10d49be
Remove unnecessary style
SeanDS Mar 27, 2020
32a376a
Rename TOCEdit to TableOfContentsEdit
SeanDS Mar 27, 2020
e49f28d
Apply suggestions from code review
SeanDS Mar 27, 2020
becb1a9
Merge branch 'add/table-of-contents' of github.com:SeanDS/gutenberg i…
SeanDS Mar 27, 2020
e7f3dae
Remove non-existent import
SeanDS Mar 27, 2020
8cbe8fc
Make imports explicit
SeanDS Mar 27, 2020
36adc5b
Remove unused function
SeanDS Mar 27, 2020
45ee5f7
Change unsubscribe function to class property
SeanDS Mar 27, 2020
04e0df2
Change JSON.stringify comparison to Lodash's isEqual
SeanDS Mar 27, 2020
afacffc
Turns out refresh() is required
SeanDS Mar 27, 2020
796900a
Remove unnecessary state setting
SeanDS Mar 27, 2020
d31534c
Don't change state on save
SeanDS Mar 27, 2020
6d1d82c
Change behaviour to only add links if there are anchors specified by …
SeanDS Mar 27, 2020
a96a5dc
Newline
SeanDS Mar 27, 2020
00255a0
Replace anchor with explicit key in map since anchor can now sometime…
SeanDS Mar 27, 2020
af0f900
Update test data
SeanDS Mar 27, 2020
9c73d7b
Update packages/block-library/src/table-of-contents/block.json
SeanDS Mar 28, 2020
99bbe0b
Rename ListLevel to ListItem for clarity
SeanDS Mar 28, 2020
d9037e1
Update packages/block-library/src/table-of-contents/ListItem.js
draganescu Apr 2, 2020
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: 2 additions & 0 deletions packages/block-library/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import * as shortcode from './shortcode';
import * as spacer from './spacer';
import * as subhead from './subhead';
import * as table from './table';
import * as tableOfContents from './table-of-contents';
import * as textColumns from './text-columns';
import * as verse from './verse';
import * as video from './video';
Expand Down Expand Up @@ -150,6 +151,7 @@ export const registerCoreBlocks = () => {
spacer,
subhead,
table,
tableOfContents,
tagCloud,
textColumns,
verse,
Expand Down
43 changes: 43 additions & 0 deletions packages/block-library/src/table-of-contents/ListItem.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/**
* WordPress dependencies
*/

Comment on lines +1 to +4
Copy link
Member

Choose a reason for hiding this comment

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

This comment should be removed.

Suggested change
/**
* WordPress dependencies
*/

export default function ListItem( props ) {
const { children, noWrapList = false } = props;
Comment on lines +5 to +6
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export default function ListItem( props ) {
const { children, noWrapList = false } = props;
export default function ListItem( { children, noWrapList = false } ) {

let childNodes = null;

if ( children ) {
childNodes = children.map( function( childNode, index ) {
Comment on lines +7 to +10
Copy link
Member

Choose a reason for hiding this comment

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

childNodes isn't used except inside the if statement, and it's never reassigned.

Suggested change
let childNodes = null;
if ( children ) {
childNodes = children.map( function( childNode, index ) {
if ( children ) {
const childNodes = children.map( function( childNode, index ) {

const { content, anchor, level } = childNode.block;

const entry = anchor ? (
<a
className="blocks-table-of-contents-entry"
Copy link
Member

Choose a reason for hiding this comment

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

This class name should follow the BEM-ish style used elsewhere in Gutenberg:

Suggested change
className="blocks-table-of-contents-entry"
className="wp-block-table-of-contents__entry"

Copy link
Member

Choose a reason for hiding this comment

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

(The tests will have to be updated.)

href={ anchor }
data-level={ level }
>
{ content }
</a>
) : (
<span
className="wp-block-table-of-contents__entry"
data-level={ level }
>
{ content }
</span>
);

return (
<li key={ index }>
{ entry }
{ childNode.children ? (
<ListItem>{ childNode.children }</ListItem>
) : null }
</li>
);
} );

// Don't wrap the list elements in <ul> if converting to a core/list.
return noWrapList ? childNodes : <ul>{ childNodes }</ul>;
}
}
17 changes: 17 additions & 0 deletions packages/block-library/src/table-of-contents/block.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"name": "core/table-of-contents",
"category": "common",
"attributes": {
"headings": {
"type": "array",
"source": "query",
"selector": ".blocks-table-of-contents-entry",
"default": [],
"query": {
"content": { "source": "text" },
"anchor": { "source": "attribute", "attribute": "href" },
"level": { "source": "attribute", "attribute": "data-level" }
}
}
}
}
72 changes: 72 additions & 0 deletions packages/block-library/src/table-of-contents/edit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/**
* External dependencies
*/

Comment on lines +1 to +4
Copy link
Member

Choose a reason for hiding this comment

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

This newline should be removed.

const { isEqual } = require( 'lodash' );

/**
* Internal dependencies
*/
import { getHeadingsList, linearToNestedHeadingList } from './utils';
import ListItem from './ListItem';
Comment on lines +7 to +11
Copy link
Member

Choose a reason for hiding this comment

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

These should come after the WordPress dependencies, not before.


/**
* WordPress dependencies
*/
import { Component } from '@wordpress/element';
import { subscribe } from '@wordpress/data';
import { __ } from '@wordpress/i18n';

class TableOfContentsEdit extends Component {
componentDidMount() {
const { attributes, setAttributes } = this.props;
let { headings } = attributes;

// Update the table of contents when changes are made to other blocks.
this.unsubscribe = subscribe( () => {
this.setState( { headings: getHeadingsList() } );
} );

if ( ! headings ) {
headings = getHeadingsList();
}

setAttributes( { headings } );
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit unclear what this does. It seems to set the headings attribute to the same value it already is. Might be possible to delete it.

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 think I had to keep that there to make the block work when newly created, but I haven't checked again since changing a bunch of other things. I can experiment removing it to see if it still behaves.

}

componentWillUnmount() {
this.unsubscribe();
}

componentDidUpdate( prevProps, prevState ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not completely sure about the need to use componentDidUpdate, it seems like this could go in the subscribe callback. It's good to reduce component updates/renders, and it seems like using componentDidUpdate will result in two successive renders.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is no longer updating state so (I think?) doesn't trigger a re-render. It might still be possible to merge into subscribe though - what do you think?

const { setAttributes } = this.props;
const { headings } = this.state;

if ( prevState && ! isEqual( headings, prevState.headings ) ) {
setAttributes( { headings } );
}
}

render() {
const { attributes } = this.props;
const { headings = [] } = attributes;

if ( headings.length === 0 ) {
return (
<p>
{ __(
'Start adding heading blocks to see a Table of Contents here'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'Start adding heading blocks to see a Table of Contents here'
'Start adding Heading blocks to create a table of contents here.'

) }
</p>
);
}

return (
<div className={ this.props.className }>
<ListItem>{ linearToNestedHeadingList( headings ) }</ListItem>
</div>
);
}
}

export default TableOfContentsEdit;
28 changes: 28 additions & 0 deletions packages/block-library/src/table-of-contents/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import edit from './edit';
import metadata from './block.json';
import save from './save';
import transforms from './transforms';

const { name } = metadata;

export { metadata, name };

export const settings = {
title: __( 'Table of Contents' ),
description: __(
'Add a list of internal links allowing your readers to quickly navigate around.'
Copy link
Member

Choose a reason for hiding this comment

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

I think this description needs to be updated since the block no longer automatically adds anchors to the 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.

True. Maybe "Add a list of headers allowing your readers to quickly navigate through your post." instead, and somewhere else make it obvious to users how to make them links (maybe with popovers or similar)?

Copy link
Member

@ZebulanStanphill ZebulanStanphill Mar 28, 2020

Choose a reason for hiding this comment

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

I'd say "headings" rather than "headers". Also, the contributor guide recommends avoiding describing features as "allowing" something.

How's this?

A list of headings to summarize your post. Add HTML anchors to Heading blocks to automatically link them here.

You might be able to leave off "automatically".

),
icon: 'list-view',
category: 'layout',
transforms,
edit,
save,
};
20 changes: 20 additions & 0 deletions packages/block-library/src/table-of-contents/save.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* Internal dependencies
*/
import { linearToNestedHeadingList } from './utils';
import ListItem from './ListItem';

export default function save( props ) {
const { attributes } = props;
const { headings } = attributes;

if ( headings.length === 0 ) {
return null;
}

return (
<nav className={ props.className }>
<ListItem>{ linearToNestedHeadingList( headings ) }</ListItem>
</nav>
);
}
31 changes: 31 additions & 0 deletions packages/block-library/src/table-of-contents/transforms.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* WordPress dependencies
*/
import { createBlock } from '@wordpress/blocks';
import { renderToString } from '@wordpress/element';

/**
* Internal dependencies
*/
import { linearToNestedHeadingList } from './utils';
import ListItem from './ListItem';

const transforms = {
to: [
{
type: 'block',
blocks: [ 'core/list' ],
transform: ( { headings } ) => {
return createBlock( 'core/list', {
values: renderToString(
<ListItem noWrapList>
{ linearToNestedHeadingList( headings ) }
</ListItem>
),
} );
},
},
],
};

export default transforms;
111 changes: 111 additions & 0 deletions packages/block-library/src/table-of-contents/utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/**
* WordPress dependencies
*/
import { select } from '@wordpress/data';
import { create } from '@wordpress/rich-text';

/**
* Takes a flat list of heading parameters and nests them based on each header's
* immediate parent's level.
*
* @param {Array} headingsList The flat list of headings to nest.
* @param {number} index The current list index.
* @return {Array} The nested list of headings.
*/
export function linearToNestedHeadingList( headingsList, index = 0 ) {
const nestedHeadingsList = [];

headingsList.forEach( function( heading, key ) {
if ( heading.content === undefined ) {
return;
}

// Make sure we are only working with the same level as the first iteration in our set.
if ( heading.level === headingsList[ 0 ].level ) {
// Check that the next iteration will return a value.
// If it does and the next level is greater than the current level,
// the next iteration becomes a child of the current interation.
if (
headingsList[ key + 1 ] !== undefined &&
headingsList[ key + 1 ].level > heading.level
) {
// We need to calculate the last index before the next iteration that has the same level (siblings).
// We then use this last index to slice the array for use in recursion.
// This prevents duplicate nodes.
let endOfSlice = headingsList.length;
for ( let i = key + 1; i < headingsList.length; i++ ) {
if ( headingsList[ i ].level === heading.level ) {
endOfSlice = i;
break;
}
}

// We found a child node: Push a new node onto the return array with children.
nestedHeadingsList.push( {
block: heading,
index: index + key,
children: linearToNestedHeadingList(
headingsList.slice( key + 1, endOfSlice ),
index + key + 1
),
} );
} else {
// No child node: Push a new node onto the return array.
nestedHeadingsList.push( {
block: heading,
index: index + key,
children: null,
} );
}
}
} );

return nestedHeadingsList;
}

/**
* Gets a list of heading texts, anchors and levels in the current document.
*
* @return {Array} The list of headings.
*/
export function getHeadingsList() {
return convertBlocksToTableOfContents( getHeadingBlocks() );
}

/**
* Gets a list of heading blocks in the current document.
*
* @return {Array} The list of heading blocks.
*/
export function getHeadingBlocks() {
const editor = select( 'core/block-editor' );
return editor
.getBlocks()
.filter( ( block ) => block.name === 'core/heading' );
}

/**
* Extracts text, anchor and level from a list of heading blocks.
*
* @param {Array} headingBlocks The list of heading blocks.
* @return {Array} The list of heading parameters.
*/
export function convertBlocksToTableOfContents( headingBlocks ) {
return headingBlocks.map( function( heading ) {
// This is a string so that it can be stored/sourced as an attribute in the table of contents
// block using a data attribute.
const level = heading.attributes.level.toString();
SeanDS marked this conversation as resolved.
Show resolved Hide resolved

const headingContent = heading.attributes.content;
const anchorContent = heading.attributes.anchor;

// Strip html from heading to use as the table of contents entry.
const content = headingContent
? create( { html: headingContent } ).text
: '';

const anchor = anchorContent ? '#' + anchorContent : '';

return { content, anchor, level };
} );
}
4 changes: 4 additions & 0 deletions packages/e2e-tests/fixtures/block-transforms.js
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,10 @@ export const EXPECTED_TRANSFORMS = {
originalBlock: 'Table',
availableTransforms: [ 'Group' ],
},
'core__table-of-contents': {
originalBlock: 'Table of Contents',
availableTransforms: [ 'Group', 'List' ],
},
'core__tag-cloud': {
originalBlock: 'Tag Cloud',
availableTransforms: [ 'Group' ],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<!-- wp:core/table-of-contents -->
<nav class="wp-block-table-of-contents"><ul><li><a class="blocks-table-of-contents-entry" href="#0-First-Heading" data-level="2">First Heading</a><ul><li><a class="blocks-table-of-contents-entry" href="#1-Sub-Heading" data-level="3">Sub Heading</a></li><li><a class="blocks-table-of-contents-entry" href="#2-Another-Sub-Heading" data-level="3">Another Sub Heading</a></li><li><span class="blocks-table-of-contents-entry" data-level="3">A Sub Heading Without Link</span></li></ul></li></ul></nav>
<!-- /wp:core/table-of-contents -->
32 changes: 32 additions & 0 deletions packages/e2e-tests/fixtures/blocks/core__table-of-contents.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
[
{
"clientId": "_clientId_0",
"name": "core/table-of-contents",
"isValid": true,
"attributes": {
"headings": [
{
"content": "First Heading",
"anchor": "#0-First-Heading",
"level": "2"
},
{
"content": "Sub Heading",
"anchor": "#1-Sub-Heading",
"level": "3"
},
{
"content": "Another Sub Heading",
"anchor": "#2-Another-Sub-Heading",
"level": "3"
},
{
"content": "A Sub Heading Without Link",
"level": "3"
}
]
},
"innerBlocks": [],
"originalContent": "<nav class=\"wp-block-table-of-contents\"><ul><li><a class=\"blocks-table-of-contents-entry\" href=\"#0-First-Heading\" data-level=\"2\">First Heading</a><ul><li><a class=\"blocks-table-of-contents-entry\" href=\"#1-Sub-Heading\" data-level=\"3\">Sub Heading</a></li><li><a class=\"blocks-table-of-contents-entry\" href=\"#2-Another-Sub-Heading\" data-level=\"3\">Another Sub Heading</a></li><li><span class=\"blocks-table-of-contents-entry\" data-level=\"3\">A Sub Heading Without Link</span></li></ul></li></ul></nav>"
}
]
Loading