-
Notifications
You must be signed in to change notification settings - Fork 299
fix(dev-hub): sidebar and table of contents positioning #3073
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
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
5 Skipped Deployments
|
overflow: hidden auto; | ||
transform: scale(calc(100% - (var(--offset) * 5%))); |
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.
@cprussin I removed these, for some reason it was messing with the position fixed/sticky and we weren't able to fix elements to the top of the page.
I'm not sure what these were doing or why we had them.
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.
This is used when opening the mobile drawer to offset the page underneath it. It does need to be here -- I'm surprised you're having issues with sticky positioning as we are actively using sticky positioning on table headers in IH to stick table headers to the top of the page....
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.
Reverted
Table of contents is still not fixed but spent too much time on this and the most obvious is making the sidebar sticky which works fine |
background: theme.color("background", "primary"); | ||
border-top-left-radius: calc(var(--offset) * theme.border-radius("xl")); | ||
border-top-right-radius: calc(var(--offset) * theme.border-radius("xl")); | ||
height: 100dvh; |
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.
I think this change is safe but I'd be careful with it and make sure to test across screen sizes, especially with the interactions with overlays / drawers
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.
actually this is just reverting to the initial version, think just the ordering changed here
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.
ah yes I missed the deleted line below, nice ok
Summary
Making dev hub sidebar and table of contents sticky for better UX
Rationale
How has this been tested?