-
-
Notifications
You must be signed in to change notification settings - Fork 66
Desktop sidebars #579
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
base: main
Are you sure you want to change the base?
Desktop sidebars #579
Conversation
✅ Deploy Preview for pauseai ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@Wituareard how would you fix that? making it go to mobile mode to the slightest zoom? |
|
Not sure, that would be one solution |
|
Also I think the library should already be able to scroll automatically? |
Mmm, what do you mean by scrolling automatically? The sidebar in the /risks page was left showing the same headers while the highlighted headers would change to be the ones out of the screen when scrolling too much (on the corpus). I'm basically vibe coding without understanding the code, but even activating a keepInView thing that the AI tried didn't fix it. |
|
Alright yeah, whatever |
|
i for the love of god cannot make it switch at other resolutions/ zooms, so i give up |
|
@joepio @Maximophone: big change, what do you think? Maybe the color of the highlighted header in the sidebar is too eye-grabbing? |
I'll try to do that |
|
I thought this change was important so I asked for reviews, let me know if it wasn't necessary |
|
Yes makes sense, thanks for the reminder |
|
I adjusted the breakpoint, but I agree that it does look a bit attention grabbing |
|
I don't see a sidebar when I go the the preview. Not sure what to look for. Also, I don't know what is the reason for these changes |
|
OK I found the context, but will need more time than I have available today to read and get up to speed |
anthonybailey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question about the approach:
I noticed the library (svelte-toc) already has built-in support for a sidebar mode. When desktop is true, it applies position: sticky and has CSS variables like --toc-desktop-sticky-top for customization.
This PR creates a separate wrapper div with position: fixed and renders a second <Toc> component for the sidebar case.
Did you try using the library's built-in desktop mode first? If so, what problem led you to this approach instead?
Not blocking — just want to understand the reasoning before we merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(dupe comment deleted)
|
I'll note also that I think the choice by svelte-toc author to call a choice desktop/mobile based on viewport width is pretty terrible, but probably not worth painful patching. |
|
i didn't try another thing |
I mean, having an index on long pages is useful; that's why I think pages like Wikipedia use them |


#571