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

Control initial expanded state of TOC; support URLs for items #248

Merged
merged 2 commits into from
May 4, 2023

Conversation

longhotsummer
Copy link
Contributor

This means the TOC can be collapsed when first rendered, and it makes it much easier to associate arbitrary URLs with a TOC item

@longhotsummer longhotsummer requested a review from goose-life May 4, 2023 06:45
@longhotsummer
Copy link
Contributor Author

This will make the peachjam taxonomy trees collapsed by default, which is much easier to navigate for complex trees.

@@ -78,6 +78,7 @@ export class TocItem {
render() {
const isParent = !!this.item.children?.length;
const showItem = !this.filteredItems || this.filteredItems.has(this.item);
const url = this.item.url || `#${this.item.id || ''}`;
Copy link
Contributor

@goose-life goose-life May 4, 2023

Choose a reason for hiding this comment

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

should the `#${ before this.item.id come out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The old mechanism for determining the URL for a TOC item was to use the id attribute to create a link like #chp_2. We're now adding support for an optional url attribute, and falling back to the old ID if it's not there. The #${...}\ is a javascript interpolation string equivalent to f'#{...}' in python. The # is literally just that, and the $ indicates the interpolation inside the curlies.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh my bad, I missed the closing } 🤦

Copy link
Contributor

@goose-life goose-life left a comment

Choose a reason for hiding this comment

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

lgtm, I just think there's a typo (I left a comment).

My only question, out of interest, is where did the id come from before, or what changed that TOCItem now needs the optional attribute?

@longhotsummer longhotsummer merged commit 32932bd into main May 4, 2023
@longhotsummer longhotsummer deleted the toc branch May 4, 2023 11:14
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