Skip to content

Responsive grid layout: fixed TopBar/Sidebar with document scroll#3178

Open
benjaminleonard wants to merge 12 commits intomainfrom
bl/responsiveness-grid-layout
Open

Responsive grid layout: fixed TopBar/Sidebar with document scroll#3178
benjaminleonard wants to merge 12 commits intomainfrom
bl/responsiveness-grid-layout

Conversation

@benjaminleonard
Copy link
Copy Markdown
Contributor

@benjaminleonard benjaminleonard commented Apr 13, 2026

Refactor of #2087

Switch from a CSS grid layout (where TopBar and Sidebar occupied grid cells alongside ContentPane) to fixed-position TopBar and Sidebar with document-level scrolling. This is the foundation for mobile/tablet support and likely the place that has the potential to cause the most issues so I'm separating the two.

  • Viewport meta tag: proper responsive width=device-width instead of fixed width=1050
  • CSS: add --sidebar-width variable, responsive --content-gutter (smaller below 1000px), remove overflow-y-hidden on body, set height: 100% on html/body/#root
  • TopBar: single fixed element (was two Fragment children occupying grid cells)
  • Sidebar: fixed position with overflow scroll
  • ContentPane: margin-left offset for sidebar, document-level scroll via min-h-[calc(100vh-...)]
  • Scroll restoration: window.scrollY instead of container .scrollTop
  • PageSkeleton: matches new fixed layout structure
  • E2e: scroll-restore test updated for document scroll, expectScrollTop/scrollTo utils use window

This PR should produce a console that is visually identical to the current one.


Broader responsive changes will come in another PR, and are not a blocker for this.

Todo

  • Fix table dropdown overlapping header
  • Double check modals are not causing layout shift

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
console Ready Ready Preview Apr 21, 2026 9:22pm

Request Review

benjaminleonard and others added 2 commits April 15, 2026 10:54
Switch from a CSS grid layout (where TopBar and Sidebar occupied grid
cells alongside ContentPane) to fixed-position TopBar and Sidebar with
document-level scroll. This is the foundation for mobile/tablet support.

Key changes:
- viewport meta tag: proper responsive instead of fixed 1050px
- CSS: add --sidebar-width var, responsive --content-gutter, remove
  body overflow-y-hidden, set html/body/#root height: 100%
- TopBar: single fixed element (was two grid cell fragments)
- Sidebar: fixed position with overflow scroll
- ContentPane: margin-left for sidebar on desktop, document scroll
- Scroll restoration: window.scrollY instead of container scrollTop
- PageSkeleton: match new fixed layout structure

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread app/components/PageSkeleton.tsx Outdated
* different points in the history stack will not share the same scroll position.
*/
export function useScrollRestoration(container: React.RefObject<HTMLElement | null>) {
export function useScrollRestoration() {
Copy link
Copy Markdown
Contributor Author

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-router one.

Copy link
Copy Markdown
Contributor Author

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-router ScrollRestoration to work

Comment thread app/layouts/helpers.tsx
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)`
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Top bar is fixed, this leaves space for it. Alternatively we use sticky for the top bar but the sidebar would still be fixed and this feels marginally cleaner.

@benjaminleonard
Copy link
Copy Markdown
Contributor Author

PR is visually identical to main with expanded visual regression tests. Haven't figured out an easy way to test it scrolled since the baseline would require a different scroll pattern to this new PR.

I'd suggest we err on merging sooner so it can be tested for longer.

@david-crespo
Copy link
Copy Markdown
Collaborator

david-crespo commented Apr 21, 2026

No visual changes — love it. Found one issue: double scroll bar on side modal when height is small enough. Probably worth updating the regression test if it didn't catch this.

https://console-git-bl-responsiveness-grid-layout-oxidecomputer.vercel.app/projects/mock-project/disks-new

image

@david-crespo
Copy link
Copy Markdown
Collaborator

david-crespo commented Apr 21, 2026

4d4e03f fixes the side modal scroll bar thing by turning on modal=true in the base UI dialog we use for both side model and regular middle modal. modal={false} a workaround for a Radix issue covered in #1745, where a native confirm() dialog stealing and returning focus triggered Radix's focus-outside handler and caused a dismiss loop.

In base-ui, this covers it and the modal=false bit is unnecessary.

<BaseDialog.Root
open={isOpen}
onOpenChange={(open, { reason }) => {
// Ignore focus-out to prevent a dismiss loop when a native confirm()
// dialog steals and returns focus (same role as the old Radix
// onFocusOutside preventDefault). See oxidecomputer/console#1745.
if (!open && reason !== 'focus-out') onDismiss()
}}

modal={false} has downsides — it disables scroll lock, focus trap, backdrop registration, and outside-content inerting — so it's nice to get rid of it.

In order to make this work I also added a scrim for the modal on top of a side modal situation with a special z-index that makes sure it's between the modal and the side modal and blocks interaction with the side modal. Right now on main you can interact with the image upload side modal while the main upload modal is going.

@david-crespo
Copy link
Copy Markdown
Collaborator

I have fixes for the e2e failures but I want to merge #3192 into main and main into this first because it affects at least one of them.

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