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

A slightly better TOC #298

Merged
merged 3 commits into from
Dec 1, 2023
Merged

A slightly better TOC #298

merged 3 commits into from
Dec 1, 2023

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Dec 1, 2023

  • add intermediate h1 sections
  • skip subtitles (h1+h2)

closes #281

- add intermediate h1 sections
- skip subtitles (h1+h2)

closes #281
@Fil Fil requested review from mbostock and cinxmo December 1, 2023 15:41
src/render.ts Outdated
@@ -124,7 +124,7 @@ interface Header {
}

function findHeaders(parseResult: ParseResult): Header[] {
return Array.from(parseHTML(parseResult.html).document.querySelectorAll("h2"))
return Array.from(parseHTML(parseResult.html).document.querySelectorAll("h1:not(:first-of-type), h2:not(h1 + h2)"))
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could create a shared file with export const selectors = ["h1:not(:first-of-type)", "h2:not(h1 + h2)"] that we can use on both the client and server?

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively it could get baked-in to the generated page code (so that the user can override it), and then passed to enableToc(selectors).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now passing it via a data-selector property in the toc. Next step would be to make it configurable but this is out of scope for this PR (and I'm not sure we should hurry on configurability).

Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

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

Nice.

@Fil Fil merged commit 22325e7 into main Dec 1, 2023
1 check passed
@Fil Fil deleted the fil/no-toc-for-subtitle branch December 1, 2023 17:22
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.

h1+h2, as a subtitle, should probably not appear in the toc?
2 participants