Add Table of Contents block #21040

SeanDS · 2020-03-20T10:05:47Z

Please see the stale PR #15426 and issue #11047 for background. This PR is simply a rebase of the changes on #15426 onto the current master - all credit to @ashwin-pc and sorta brilliant. As of writing this contains only the changes made there, with only tiny changes to comments. There are likely some bugs probably still need ironed out. The text below is copied from that PR.

Code contributions in this commit entirely made by ashwin-pc, originally based on the
"Guidepost" block by sorta brilliant (https://sortabrilliant.com/guidepost/).

Description

Adding a Table of Contents block. #11047

This block is based off a similar block plugin Guidepost but rebuilt to essentially behave similar to Table of Contents as seen on other major platforms such as Google Docs and Word.

The block auto populates links based on the heading hierarchy of the post with the option to either:

auto-sync changes

or to sync changes when manually triggered.

When auto sync is turned off the block also allows you to manually edit the link names similar to other Table of Contents implementations.

How has this been tested?

Passed all unit tests using npm run test and all end to end tests using npm run test-e2e. new Unit Tests will have to be written for the new block though.

Screenshots

Already given above.

Types of changes

New feature (non-breaking change which adds functionality).

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.

Adds Table of Contents block to the editor. Code contributions in this commit entirely made by ashwin-pc, originally based on the "Guidepost" block by sorta brilliant (https://sortabrilliant.com/guidepost/).

SeanDS · 2020-03-20T10:11:44Z

packages/block-library/src/table-of-contents/utils.js

+		let anchor = contentDiv.textContent || contentDiv.innerText || '';
+
+		if ( anchor !== '' && anchor.indexOf( '#' ) === -1 ) {
+			anchor = '#' + cleanForSlug( anchor );


I noticed a bug: this function doesn't strip off HTML tags so when the title contains HTML these end up in the anchor:

Any suggestions for a built-in Gutenberg/JavaScript function to use to properly slugify the anchor?

It looks like that's what lines 76-83 try to do. If it's a RichText value (which it should be from the headings block) I think you can replace those lines with this:

const plainTextContent = content ? create( { html: content } ).text : '';

(create is from the RichText package).

Looking again, this is most likely a bug in the updateHeadingBlockAnchors function that automatically sets the anchor value. That would also need to convert the content to plain text.

I've implemented your code, but wouldn't getTextContent from RichText do the same job?

I wonder if we want to allow the HTML style from the header to copy over into the table of contents entry too.

ZebulanStanphill · 2020-03-20T17:31:44Z

With regard to the auto-sync behavior, I think it's a bit of a weird way to handle custom-named links. I think it would be better if the auto-sync was always on, and you could simply convert the block to a List block if you wanted to start customizing the names. This would be a one-way conversion, of course, but re-enabling the auto-sync behavior on the Table of Contents block would wipe out your customizations anyway, so it doesn't matter.

packages/block-library/src/table-of-contents/utils.js

Co-Authored-By: Zebulan Stanphill <[email protected]>

SeanDS · 2020-03-20T19:33:34Z

@ZebulanStanphill

With regard to the auto-sync behavior, I think it's a bit of a weird way to handle custom-named links. I think it would be better if the auto-sync was always on, and you could simply convert the block to a List block if you wanted to start customizing the names. This would be a one-way conversion, of course, but re-enabling the auto-sync behavior on the Table of Contents block would wipe out your customizations anyway, so it doesn't matter.

Yes I agree, I didn't like that change from the original Guidepost plugin behaviour, which just didn't let you edit the contents. I like your idea of allowing it to be changed one way to a list though.

Maybe it'd be good to get feedback from others before making such a large change in behaviour though.

paaljoachim · 2020-03-21T07:16:46Z

I just tested this out by using gutenberg.run (it took a few tries to get it to run)
I really liked what I saw. This block will be very helpful for longer articles.

Going through the process.

Finding the block.

Block settings

Turning off Auto Sync. Renaming a header link.

Clicking Auto Sync

Auto syncing removes any renaming of the Header links.

It would be nice to auto sync and at the same time keep the name customizations in place without these having to revert to the original heading name.

Perhaps a check box in the sidebar settings. Something like this. Having a checkbox with text that says: "Do not sync custom header names" or something to that effect.

Btw the block can perhaps also be transformed into a List Block as Zeb mentions.

Anyhow we should have someone from design go through this PR.
@mapk @jasmussen

Good job Sean! (Edit: Good followup job..:)

SeanDS · 2020-03-21T11:10:52Z

@paaljoachim thanks for checking it out, though almost all credit should go to the previous PR author and the original authors of Guidepost!

I'd indeed be keen to hear from design reviewers before making any major changes.

mapk · 2020-03-23T23:31:52Z

I tested this today and noticed a bug. I wasn't sure how I got it, but I dropped some ipsum on my page, created a few headings in the middle, then came back to the top to create another heading. I ended up with this:

As you can see, the first heading listed in my Table of Contents block isn't on my page. I'm not sure why it's there or how to get rid of it. I have "auto sync" turned "on."

paaljoachim · 2020-03-24T11:16:32Z

I retested with using gutenberg.run

Went here: https://lipsum.com/
Added 5 paragraphs.
Added the Table of Contents Block.

Backend result:

Frontend result:

draganescu

Great work rebasing and refreshing @SeanDS, generally this looks and works very nice. Left some code related comments. Also:

when turning off AutoSync and then editing the text of the links the post fails to update

the LinkControl when clicking on the link button in the block's toolbar is empty. This is tricky as the LinkControl does not support anchors yet.

we should remove some of the formatting options available (code, image, strikethrough)

packages/block-library/src/table-of-contents/utils.js

packages/block-library/src/table-of-contents/edit.js

packages/block-library/src/table-of-contents/utils.js

packages/block-library/src/table-of-contents/editor.scss

packages/block-library/src/table-of-contents/ListLevel.js

Copy changes from pull request #15426 (#15426). Adds Table of Contents block to the editor. Code contributions in this commit entirely made by ashwin-pc, originally based on the "Guidepost" block by sorta brilliant (https://sortabrilliant.com/guidepost/). Apply polish suggestions from code review. Improve variable names. Add comment Get rid of autosync (users should now convert to list if they want to edit the contents) Add ability to transform into list; remove unused ListLevel props Update table-of-contents block test configuration Simplify expression Remove unused function Remove unused styles. Rename TOCEdit to TableOfContentsEdit Apply suggestions from code review Remove non-existent import Make imports explicit Remove unused function Change unsubscribe function to class property Change JSON.stringify comparison to Lodash's isEqual Turns out refresh() is required Remove unnecessary state setting Don't change state on save Change behaviour to only add links if there are anchors specified by the user Newline Replace anchor with explicit key in map since anchor can now sometimes be empty Update test data Update packages/block-library/src/table-of-contents/block.json Rename ListLevel to ListItem for clarity and polish. Co-authored-by: ashwin-pc <[email protected]> Co-authored-by: Daniel Richards <[email protected]> Co-authored-by: Zebulan Stanphill <[email protected]>

Copy changes from pull request #15426 (#15426). Adds Table of Contents block to the editor. Code contributions in this commit entirely made by ashwin-pc, originally based on the "Guidepost" block by sorta brilliant (https://sortabrilliant.com/guidepost/). Apply polish suggestions from code review. Improve variable names. Add comment Get rid of autosync (users should now convert to list if they want to edit the contents) Add ability to transform into list; remove unused ListLevel props Update table-of-contents block test configuration Simplify expression Remove unused function Remove unused styles. Rename TOCEdit to TableOfContentsEdit Apply suggestions from code review Remove non-existent import Make imports explicit Remove unused function Change unsubscribe function to class property Change JSON.stringify comparison to Lodash's isEqual Turns out refresh() is required Remove unnecessary state setting Don't change state on save Change behaviour to only add links if there are anchors specified by the user Newline Replace anchor with explicit key in map since anchor can now sometimes be empty Update test data Update packages/block-library/src/table-of-contents/block.json Rename ListLevel to ListItem for clarity and polish. Co-authored-by: ashwin-pc <[email protected]> Co-authored-by: Daniel Richards <[email protected]> Co-authored-by: Zebulan Stanphill <[email protected]> Co-authored-by: JR Tashjian <[email protected]>

* SeanDS's implementation #21040 (#21040). Copy changes from pull request #15426 (#15426). Adds Table of Contents block to the editor. Code contributions in this commit entirely made by ashwin-pc, originally based on the "Guidepost" block by sorta brilliant (https://sortabrilliant.com/guidepost/). Apply polish suggestions from code review. Improve variable names. Add comment Get rid of autosync (users should now convert to list if they want to edit the contents) Add ability to transform into list; remove unused ListLevel props Update table-of-contents block test configuration Simplify expression Remove unused function Remove unused styles. Rename TOCEdit to TableOfContentsEdit Apply suggestions from code review Remove non-existent import Make imports explicit Remove unused function Change unsubscribe function to class property Change JSON.stringify comparison to Lodash's isEqual Turns out refresh() is required Remove unnecessary state setting Don't change state on save Change behaviour to only add links if there are anchors specified by the user Newline Replace anchor with explicit key in map since anchor can now sometimes be empty Update test data Update packages/block-library/src/table-of-contents/block.json Rename ListLevel to ListItem for clarity and polish. Co-authored-by: ashwin-pc <[email protected]> Co-authored-by: Daniel Richards <[email protected]> Co-authored-by: Zebulan Stanphill <[email protected]> Co-authored-by: JR Tashjian <[email protected]> * Polish, use hooks, and half-fix undo behavior. * Make dynamic, and add support for paginated posts + h1-h6 tags outside of core blocks. DRY out usage of list item class name. Improve handling of heading attributes. Add proper placeholder state in editor. Fix mistakes, improve naming/description, and polish code. Add support for page breaks in a Classic block. Always ignore empty headings. Various performance improvements. Change List block conversion to be a button in the toolbar. (It can't use the transform API since it uses dynamic data.) Remove unused key from hierarchical heading list. * Add unique icon. Co-authored-by: Joen A <[email protected]> * Fix "Convert to static list" control. * Tweak keywords. Co-authored-by: Sean Leavey <[email protected]> Co-authored-by: ashwin-pc <[email protected]> Co-authored-by: Daniel Richards <[email protected]> Co-authored-by: JR Tashjian <[email protected]> Co-authored-by: Joen A <[email protected]>

Copy changes from pull request WordPress#15426 (WordPress#15426).

e5bea61

Adds Table of Contents block to the editor. Code contributions in this commit entirely made by ashwin-pc, originally based on the "Guidepost" block by sorta brilliant (https://sortabrilliant.com/guidepost/).

SeanDS requested review from ajitbohra, jorgefilipecosta, nerrad, ntwb, Soean and talldan as code owners March 20, 2020 10:05

This was referenced Mar 20, 2020

Add/table of contents #15426

Closed

"Table of Contents" Block #11047

Closed

SeanDS commented Mar 20, 2020

View reviewed changes

ZebulanStanphill reviewed Mar 20, 2020

View reviewed changes

SeanDS and others added 5 commits March 20, 2020 19:30

Update packages/block-library/src/table-of-contents/utils.js

ba40205

Co-Authored-By: Zebulan Stanphill <[email protected]>

Update packages/block-library/src/table-of-contents/utils.js

ba5c43a

Co-Authored-By: Zebulan Stanphill <[email protected]>

Update packages/block-library/src/table-of-contents/utils.js

8507968

Co-Authored-By: Zebulan Stanphill <[email protected]>

Update packages/block-library/src/table-of-contents/utils.js

50df5b5

Co-Authored-By: Zebulan Stanphill <[email protected]>

Update packages/block-library/src/table-of-contents/utils.js

4c650b5

Co-Authored-By: Zebulan Stanphill <[email protected]>

draganescu requested changes Mar 24, 2020

View reviewed changes

ZebulanStanphill reviewed Mar 24, 2020

View reviewed changes

packages/block-library/src/table-of-contents/utils.js Show resolved Hide resolved

ZebulanStanphill reviewed Mar 24, 2020

View reviewed changes

packages/block-library/src/table-of-contents/editor.scss Outdated Show resolved Hide resolved

ZebulanStanphill reviewed Mar 24, 2020

View reviewed changes

packages/block-library/src/table-of-contents/ListLevel.js Outdated Show resolved Hide resolved

SeanDS added 4 commits March 25, 2020 00:16

Merge branch 'master' into add/table-of-contents

371a901

Merge branch 'master' into add/table-of-contents

d2e929b

Fix variable name capitalisation

c0f2e1a

Rename function

4fce9a1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Table of Contents block #21040

Add Table of Contents block #21040

SeanDS commented Mar 20, 2020

SeanDS Mar 20, 2020

talldan Mar 27, 2020

SeanDS Mar 27, 2020

ZebulanStanphill commented Mar 20, 2020

SeanDS commented Mar 20, 2020 •

edited

Loading

paaljoachim commented Mar 21, 2020 •

edited

Loading

SeanDS commented Mar 21, 2020

mapk commented Mar 23, 2020

paaljoachim commented Mar 24, 2020

draganescu left a comment •

edited

Loading

Add Table of Contents block #21040

Add Table of Contents block #21040

Conversation

SeanDS commented Mar 20, 2020

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

SeanDS Mar 20, 2020

Choose a reason for hiding this comment

talldan Mar 27, 2020

Choose a reason for hiding this comment

SeanDS Mar 27, 2020

Choose a reason for hiding this comment

ZebulanStanphill commented Mar 20, 2020

SeanDS commented Mar 20, 2020 • edited Loading

paaljoachim commented Mar 21, 2020 • edited Loading

SeanDS commented Mar 21, 2020

mapk commented Mar 23, 2020

paaljoachim commented Mar 24, 2020

draganescu left a comment • edited Loading

Choose a reason for hiding this comment

SeanDS commented Mar 20, 2020 •

edited

Loading

paaljoachim commented Mar 21, 2020 •

edited

Loading

draganescu left a comment •

edited

Loading