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: Second ToC element not collapsible or highlight-able #1505

Open
wants to merge 1 commit into
base: v4
Choose a base branch
from

Conversation

Stephen-X
Copy link
Contributor

@Stephen-X Stephen-X commented Oct 12, 2024

Here's a quick and dirty fix to ToC to make sure when multiple ToCs are added to the DOM (either visibly or not when say Component.DesktopOnly() is used), the rest of the ToC elements can be collapsed or title-highlighted.

Screenshot 2024-10-12 at 1 21 11 PM

ToC is useful for navigating long blog posts, but because it's typically to the right, it'd get pushed down to the bottom or hidden in tablet and mobile views. I feel adding multiple ToCs (e.g. one in beforeBody one in right) may be a common scenario worth supporting.

This fix might be a bit hacky, but I don't know how to generate dynamic IDs and deal with ARIA and styling otherwise. Please feel free to supersede this PR with a better fix.

A note about other components that are broken when added twice

Explorer is another obvious example, if one is to add a second one in afterBody for the mobile view, that one is not collapsible.

But I'd think #1471 is a much better solution for Explorer. Perhaps in the future it can be extended so that the hamburger menu becomes a new Quartz layout component (although I'd think double-adding components would become more of an issue to be solved then; it was briefly discussed in the original layout rework PR I was looking forward to: #1339).


EDIT: This PR appears to overlap with #1338 a bit. 1338 fixes the Explorer and ToC's collapse function, but doesn't fix ToC's title highlight function.

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.

1 participant