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

Fix: Table of contents refactor #540

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DiegoxK
Copy link

@DiegoxK DiegoxK commented Feb 6, 2024

Description

I usually organize my Notion documents into two columns: one for the content itself and another for the table of contents.

To my surprise, the headers from the left column were not appearing in the table of contents:
Screenshot 2024-02-06 173013

Upon inspecting the code, I realized that the getPageTableOfContents function only checks for the nearest children blocks of the root page and doesn't inspect sub-blocks for headers.

My changes adds the traverseBlocks function to the get-page-table-of-contents file, which recursively checks every block for headers:

Screenshot 2024-02-06 183837

Following these changes, the table of contents displays all the headers in the same way Notion does when using the two column blocks.

Notion Test Page ID

159efb60502b435496e6e37ef8e2ed6f

I'm uncertain of any potential performance impacts by checking every block this way rather than directly filtering the recordMap. Any feedback is appreciated! ❤️

Copy link

vercel bot commented Feb 6, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
react-notion-x ✅ Ready (Inspect) Visit Preview Feb 6, 2024 11:48pm
react-notion-x-minimal-demo ✅ Ready (Inspect) Visit Preview Feb 6, 2024 11:48pm

@rimonhanna
Copy link
Contributor

@DiegoxK can you rebase and solve the merge conflicts? thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants