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

feature: Table of Contents #202

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

feature: Table of Contents #202

wants to merge 8 commits into from

Conversation

jbmoelker
Copy link
Member

@jbmoelker jbmoelker commented Nov 5, 2024

Changes

  • Adds unique html id slugs to heading elements.
  • Extracts these headings and turns them into a table of contents with links to the headings.

Result:

image

Associated issue

N/A

How to test

Checklist

  • I have performed a self-review of my own code
  • I have made sure that my PR is easy to review (not too big, includes comments)
  • I have made updated relevant documentation files (in project README, docs/, etc)
  • I have added a decision log entry if the change affects the architecture or changes a significant technology
  • I have notified a reviewer

- Adds unique html id slugs to heading elements.
- Extracts these headings and turns them into a table of contents with links to the headings.
Copy link

cloudflare-workers-and-pages bot commented Nov 5, 2024

Deploying head-start with  Cloudflare Pages  Cloudflare Pages

Latest commit: 171d071
Status:🚫  Build failed.

View logs

package.json Show resolved Hide resolved
@@ -0,0 +1,18 @@
import GithubSlugger from 'github-slugger';
Copy link
Member Author

Choose a reason for hiding this comment

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

Placing this script and export the resetHtmlIdSlugger in the astro file caused the compiler not to be able to find slugger. Extracting it to this file solves that issue. But the new problem is that now this script isn't really scoped to the provider anymore.

May need to use a different mechanism, like https://docs.astro.build/en/recipes/sharing-state/ ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Or maybe we could use an actual context with this library: https://withastro-utils.github.io/docs/guides/context/

slugger.reset();
};

export const htmlIdSlug = ({ id, html }: { id?: string; html: string }) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

should add test coverage for these methods

@jbmoelker jbmoelker added the help wanted Extra attention is needed label Nov 27, 2024
@jbmoelker
Copy link
Member Author

jbmoelker commented Dec 21, 2024

Implementation is blocked as the AstroContainer's renderToString method relies on node:path, which is not available in the Cloudflare runtime:

[ERROR] [vite] x Build failed in 1.25s
[commonjs--resolver] [plugin vite:resolve] Cannot bundle Node.js built-in "node:path" imported from "node_modules/astro/dist/core/config/schema.js". Consider disabling ssr.noExternal or remove the built-in dependency.

Help wanted: can we render the blocks using a different method? Or should we abandon this feature?

Update: resolving this with slots.render?

However, this problem remains:
```
[ERROR] [vite] x Build failed in 1.25s
[commonjs--resolver] [plugin vite:resolve] Cannot bundle Node.js built-in "node:path" imported from "node_modules/astro/dist/core/config/schema.js". Consider disabling ssr.noExternal or remove the built-in dependency.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant