ToC block: re-enable + use static markup and only support core Heading and Page Break blocks #29739
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ZebulanStanphill
added
the
[Block] Table of contents (experimental)
This comment has been minimized.
This comment has been minimized.
7 tasks
|
Size Change: +1.35 kB (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
ZebulanStanphill
added
the
Needs Technical Feedback
ZebulanStanphill
force-pushed
the
update/toc-block-use-static-markup
branch
2 times, most recently
from
9b7fcff to
cd93c8f
Compare
3 tasks
ZebulanStanphill
force-pushed
the
update/toc-block-use-static-markup
branch
from
cd93c8f to
4ec45cf
Compare
|
@ZebulanStanphill I have also problem with infinite loop in the edit.js. I narrowed it down to setHeadingTree function in edit.js. Somehow setting headingTree fires loop. I couldn't find out why. Will check again tomorrow. |
ZebulanStanphill
force-pushed
the
update/toc-block-use-static-markup
branch
4 times, most recently
from
348f581 to
0dfa131
Compare
ZebulanStanphill
requested review from
ajitbohra,
nerrad,
ntwb,
spacedmonkey and
TimothyBJacobs
as code owners
|
Alright, I finally managed to figure out what was causing an infinite loop: it was simply a result of trying to use |
ZebulanStanphill
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Are any of these issues something you would be interested in? |
|
@mcsf @jorgefilipecosta @aristath |
7 tasks
Co-authored-by: Miguel Fonseca <miguelcsf@gmail.com>
Massive props to @mcsf for figuring this out. Co-authored-by: Miguel Fonseca <miguelcsf@gmail.com>
…es in the codebase.
ZebulanStanphill
force-pushed
the
update/toc-block-use-static-markup
branch
from
b84f91e to
327338f
Compare
|
@adamziel Done! |
adamziel
approved these changes
May 6, 2022
There was a problem hiding this comment.
This PR went through a lot of iterations and been sitting here for a while. As of now, it seems to be in a good enough shape to get it merged, especially since the block is still experimental. Let's figure out any kinks in follow-up PRs. Good job @ZebulanStanphill!
|
Cool to see this getting a new cycle of life. Let's see how it works and refine |
|
Is there a simple way to check this feature? |
|
@mahnunchik Once Gutenberg 13.3 comes out, it should be as easy as inserting the block alongside some Heading and Page Break blocks and playing around with it from there. (The recently released 13.2 does not contain the block.) Or do you mean "check" as in set up functional tests for the complex behaviors of the block? Unfortunately, there currently aren't any of those set up for the block. I've gone ahead and opened an issue to track progress on that front: #41032. |
7 tasks
|
Will wordpress 6.1 have gutenberg 13.3, aka this new block? :) |
|
Since I am using this on Wordpress.com, I have some thoughts. It would be very useful if I could specify which headings level to include in the TOC. Either directly the range as "From: [h2] To: [h3]" or at least "Up to: [h2]". |
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes #29560 and re-enables the Table of Contents block, which was first merged over a year ago in #21234, only to be temporarily (heh) disabled 2 weeks later in #29718 to provide more time for some implementation issues to be resolved.
This time, the block is using static markup and only supports the core Heading and Page Break blocks, using a much cleaner and performant implementation. I've also switched to using
<ol>for the lists.Implementation notes
Why
<ol>s?See #29739 (comment) and this other comment for additional notes on that decision.
Why absolute links?
See #29739 (comment).
Undo behavior quirk
I have noticed one undesirable quirk with the current implementation, though I'm not sure if there's any way around it: when a Table of Contents block is present, every letter typed in a Heading block counts as a separate undo step, in contrast to the default behavior of multiple letters typed in sequence being combined into a single undo step. Is this a blocker? If so, I'd appreciate any suggestions on how to fix it, because I'm not aware of any solutions here. (Remember, it only affects Headings, not other blocks.)
I've opened #41031 to track this bug.
Static rendering
This PR currently uses static rendering for the Table of Contents, meaning the markup is only altered in the editor, and on save, the final markup is saved to the post content.
Advantages
Disadvantages
Testing instructions
Try adding Heading and Page Break blocks to create a paginated post. Set various heading levels on the Headings. Set anchors/ids on some of the Headings. Place a Table of Contents block somewhere between the Page Breaks. Ensure that the links to the headings work on the front-end, regardless of which page the headings are on. (Note that only headings with anchors will be linked.) Try toggling the "Only include current page" option to make the block only list headings on the same page as itself.
Note that in a recent commit, I've added a feature where inserting a Table of Contents block turns on the Heading block's automatic anchor generation - a feature which is already in core, but is disabled by default, with no visible UI control available to turn it on permanently yet.
Screenshot
Checklist: