refactor(components): replace useResizeObserver with CSS container queries#2939
refactor(components): replace useResizeObserver with CSS container queries#2939
Conversation
Both the legacy and composable Page header now use a Container component to establish a container query context instead of measuring titlebar width in JS and applying breakpoint classes. This eliminates the layout flash on initial render (where the header briefly showed the wide layout before JS measured the actual width) and removes a React re-render on every resize.
Deploying atlantis with
|
| Latest commit: |
d28b8ef
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ddd46429.atlantis.pages.dev |
| Branch Preview URL: | https://nazarii-switch-page-to-conta.atlantis.pages.dev |
|
we should add tests for all the breakpoint sizes. preferably we do this in a separate PR so it can land in master, and then we'd see failures on this branch that verify it would catch the missing layout. |
| margin: 0; | ||
| flex: 0 auto; | ||
| } | ||
| @container page-titlebar (max-width: 499px) { |
There was a problem hiding this comment.
we have some CSS vars that we can use here to stay in sync with how we're treating responsive sizes throughout the design system. let's use those!
We have 5 CSS breakpoints that you can choose from when styling your components.
| Breakpoint | Width |
| :----------------------------- | :---------- |
| `--small-screens-and-below` | `< 490px` |
| `--small-screens-and-up` | `>= 490px` |
| `--medium-screens-and-up` | `>= 768px` |
| `--large-screens-and-up` | `>= 1080px` |
| `--extra-large-screens-and-up` | `>= 1440px` |
the values match what useBreakpoints would have provided
There was a problem hiding this comment.
disclaimer: I know this CSS was more or less like this before.
I find it really hard to follow and see what kind of a layout we are trying to achieve with all this. I'm tempted to do a small refactor that uses CSS grid. lemme put together a quick code sample to show how that might work and look.
There was a problem hiding this comment.
|
this isn't strictly related to this PR. more so something I missed in the last one, but can we remove most of these exports in the index.ts? I'd prefer not to expose "legacy" or "slot". these aren't terms we are ready to commit to or want to reference externally. I'd like to keep the exports down to the bare minimum required really. |
| fullWidth={true} | ||
| type="secondary" | ||
| /> | ||
| <Container name="page-titlebar"> |
There was a problem hiding this comment.
I'm kind of torn on this.
on one hand, it is nice to use our own components such as Container
on the other, we already have titleBar which needs its own styles. so we could easily make the container query definition live in there.... though actually, what benefit do we have from using the titlebar?
I would have thought it would be the outermost container, the one that applies pageStyles. to me that seems more resilient and stable. we're always going to have a container that fills the width. I'm not so confident with the titlebar.
sorry that was an aside, getting back to my original point - either on pageStyles or titleBar we need these classes and divs no matter what. why not re-use them and apply the container styles in there?
by using Container we're adding another layer of div nesting and it's not immediately clear to me what benefit it offers aside from using our own components to build a layout.... the thing is, that doing it that way adds a lot of nesting to a layout.









Motivations
Pagepreviously useduseResizeObserverto measure the titlebar's pixel width at runtime and apply one of three breakpoint CSS classes (small,medium,large) to control the responsive layout of the title and action buttons. This meant the header rendered with the default (wide) layout on every first paint, then snapped to the correct layout after JavaScript measured the actual width — causing a visible layout shift. It also triggered a React re-render on every resize event.CSS Container Queries let the browser handle this responsively in a single CSS pass, with no JS measurement, no layout flash, and no re-renders on resize.
This is a re-attempt of #2845, which was reverted in #2875 due to CSS bugs in the original implementation. This PR fixes those bugs and applies the migration to both the legacy and composable
PageAPIs.Changes
Added
@container page-titlebar (max-width: 499px)rule: stacks action buttons to full-width with correct vertical gap between adjacent buttons.@container page-titlebar (min-width: 500px)rule: switches the titlebar to inline-flex, makes the action groupnowrapandjustify-end, and sizes buttons to content width.Changed
PageLegacyandPageHeader(composable API) now wrap the titlebardivin<Container name="page-titlebar">to establish the container query context, instead of measuring width in JS.display: flex; flex-wrap: wrap; justify-content: space-betweenmoved directly onto.titleBar > .actionGroup(was conditionally applied via JS class).Removed
useResizeObserverandBreakpointsimport from@jobber/hooksinPage.tsx.titleBarClassescomputation andtitleBarRefin bothPageLegacyandPageHeader..small,.medium,.largeand their associated layout rules fromPage.module.css.Page.module.css.d.ts.jest.mock("@jobber/hooks", ...)block fromPage.test.tsxthat fakeduseResizeObserverreturn values (no longer needed since layout is pure CSS).Fixed
Testing
cd packages/components && npx jest Page— all pass with the mock removed.cd packages/site && npx playwright test tests/visual/page.titlebar.visual.ts— all pass. Snapshots have been updated.