-
Notifications
You must be signed in to change notification settings - Fork 23
Responsive grid layout: fixed TopBar/Sidebar with document scroll #3178
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
Merged
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
41c10c1
Responsive grid layout: fixed TopBar/Sidebar with document scroll
benjaminleonard ecf1870
Fix missing brace
benjaminleonard 5bce8f8
Expand visual regression tests
benjaminleonard 680e6c5
Remove `scroll-container` and references
benjaminleonard 979e8e3
Remove unnecessary overscroll style
benjaminleonard 33e9efa
Test fix
benjaminleonard e0e7e52
zIndex prop on dropdown
benjaminleonard e28ae94
Prevent overscroll on modals and sidebar
benjaminleonard 5d810ff
Add collision padding to the action menu (avoids pagination)
benjaminleonard 25df9c7
Share topBar and sidebar classes with skeleton
benjaminleonard b4a833a
only need one :root
david-crespo 4d4e03f
modal=true on baseui dialogs, fix scrim behind nested image upload modal
david-crespo 2ef0688
merge main
david-crespo 8e89743
fix failing e2e tests
david-crespo 9881dd5
write e2e test output to .e2e-logs for agents
david-crespo 9faa577
opt out of native scroll restore
david-crespo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,6 @@ | |
| * | ||
| * Copyright Oxide Computer Company | ||
| */ | ||
| import { useRef } from 'react' | ||
| import { Outlet } from 'react-router' | ||
|
|
||
| import { PageActionsTarget } from '~/components/PageActions' | ||
|
|
@@ -14,18 +13,18 @@ import { useScrollRestoration } from '~/hooks/use-scroll-restoration' | |
| import { SkipLinkTarget } from '~/ui/lib/SkipLink' | ||
| import { classed } from '~/util/classed' | ||
|
|
||
| export const PageContainer = classed.div`grid h-screen grid-cols-[14.25rem_1fr] grid-rows-[var(--top-bar-height)_1fr]` | ||
| export const PageContainer = classed.div`min-h-full pt-(--top-bar-height)` | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Top bar is fixed, this leaves space for it. Alternatively we use |
||
|
|
||
| // shared with PageSkeleton so the skeleton doesn't drift from the real layout | ||
| export const topBarWrapperClass = | ||
| 'bg-default border-secondary fixed top-0 right-0 left-0 z-(--z-top-bar) grid h-(--top-bar-height) grid-cols-[var(--sidebar-width)_1fr] border-b' | ||
| export const sidebarWrapperClass = | ||
| 'border-secondary fixed top-(--top-bar-height) bottom-0 left-0 w-(--sidebar-width) border-r' | ||
|
|
||
| export function ContentPane() { | ||
| const ref = useRef<HTMLDivElement>(null) | ||
| useScrollRestoration(ref) | ||
| useScrollRestoration() | ||
| return ( | ||
| <div | ||
| ref={ref} | ||
| className="light:bg-raise flex flex-col overflow-auto" | ||
| id="scroll-container" | ||
| data-testid="scroll-container" | ||
| > | ||
| <div className="light:bg-raise ml-(--sidebar-width) flex min-h-[calc(100vh-var(--top-bar-height))] flex-col"> | ||
| <div className="flex grow flex-col pb-8"> | ||
| <SkipLinkTarget /> | ||
| <main className="*:gutter"> | ||
|
|
@@ -47,12 +46,10 @@ export function ContentPane() { | |
| * `<div>` because we don't need it. | ||
| */ | ||
| export const SerialConsoleContentPane = () => ( | ||
| <div className="flex flex-col overflow-auto"> | ||
| <div className="flex grow flex-col"> | ||
| <SkipLinkTarget /> | ||
| <main className="*:gutter h-full"> | ||
| <Outlet /> | ||
| </main> | ||
| </div> | ||
| <div className="ml-(--sidebar-width) flex h-[calc(100vh-var(--top-bar-height))] flex-col overflow-hidden"> | ||
| <SkipLinkTarget /> | ||
| <main className="*:gutter h-full"> | ||
| <Outlet /> | ||
| </main> | ||
| </div> | ||
| ) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Do we even need this hook now? Since we're using a regular window scroll, perhaps we can use the built-in
react-routerone.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.
Similar to #2450 if I remove it I was unable to get
react-routerScrollRestorationto work