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

Toc component: style improvements #405

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

DanielWTQ
Copy link
Contributor

Improve the styles for the newly added Toc component, which adds a sticky table of contents:

  • Restore a background color for the table.

  • With that background color, the table shouldn't take up the entire width of the page, so remove the 100% width.

  • Specify z-index: 1; for the wrapping div so that reference tooltips from the Cite extension are not shown through the table of contents.

  • Restore the ability to toggle the visibility of the table of contents.

WE-279

@malberts
Copy link
Contributor

What is your use case here, specifically regarding the background color and width? It sounds like you still have the TOC in the bigger main content area?

In my original use case the TOC goes into the sidebar:
Screenshot_20231129_130423

But with the default background and without the width (this PR):
Screenshot_20231129_130218

I'm happy to find a sensible default that makes sense for more use cases. Right now I don't consider the TOC component to be official yet, so we can change things.

@DanielWTQ
Copy link
Contributor Author

@malberts so as you guessed we were having the table of contents in the main contents area, in fact to test this out I was using the standard.xml layout but adding <component type="Toc" /> right before <component type="MainContent"/>.

If you don't mind sharing the layout that you were using, I can rework this patch to ensure that the sidebar styles aren't affected by either moving the styles for the sidebar to be scoped to the sidebar, or moving my changes to be scoped to the main content section

@malberts
Copy link
Contributor

@DanielWTQ I posted a very basic sidebar example here: #386 (comment)

@malberts
Copy link
Contributor

I moved the Hide/Show fix into a separate PR: #410. Thanks for including it here.

Improve the styles for the newly added Toc component, which adds a sticky table
of contents:

- Restore a background color for the table.

- With that background color, the table shouldn't take up the entire width of
the page, so remove the 100% width.

- Specify `z-index: 1;` for the wrapping div so that reference tooltips from
the Cite extension are not shown through the table of contents.

WE-279
@DanielWTQ
Copy link
Contributor Author

I moved the Hide/Show fix into a separate PR: #410. Thanks for including it here.

Thanks - I removed that from my patch. Please let me know what else needs to be fixed for this to be mergeable

@malberts
Copy link
Contributor

My original concern about the background color and width still apply. What is the use case for putting the TOC in the main section? Are you trying to separate it from the page content somehow?

@DanielWTQ
Copy link
Contributor Author

My original concern about the background color and width still apply. What is the use case for putting the TOC in the main section? Are you trying to separate it from the page content somehow?

I was putting it in the main section to be above the normal page content but still in the same horizontal area, the way that tables of contents exist in Vector, for example.

@malberts malberts merged commit 44d0d33 into ProfessionalWiki:master Jul 29, 2024
4 checks passed
@malberts
Copy link
Contributor

I'm merging this, but I'll add a note that the wiki admin should should implement styling since the (likely more common) use case of moving the TOC to the sidebar will look ugly otherwise. I cannot think of an easy way to distinguish the TOC being in the sidebar, above MainContent, or anywhere else really.

@DanielWTQ DanielWTQ deleted the stickytoc branch July 31, 2024 20:06
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