ref(replays): Migrate responsive styles to container queries - #120643
Conversation
Make replay components respond to their available container width while preserving existing layout thresholds. Refs #120315
github-actions
Bot
added
the
Scope: Frontend
Story previewsPreview the stories changed in this PR on the Vercel deployment: Preview deployment: https://sentry-nh0lzf909.sentry.dev |
…gration Two of the migrated rules can't resolve against a container: - The timeline event tooltip is portaled to the body, which has no query container, so the @container clamp never matched. Restore the media query. - The replay metadata dl is a flex item, so justify-self never applied on either axis. Drop the rule. Refs #120315
|
|
||
| @media (min-width: ${p => p.theme.breakpoints.md}) { | ||
| justify-self: flex-end; | ||
| } |
There was a problem hiding this comment.
justify-self has no effect on flex items, and this dl is a direct child of a <Flex>, so the rule never applied.
There was a problem hiding this comment.
In flexbox layouts, this property is ignored. Read more about alignment in flexbox.
https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Properties/justify-self
2fc35f0 to
8f348e7
Compare
| name ?? '', | ||
| version && isLargeBreakpoint ? version : undefined | ||
| ); | ||
| const icon = generatePlatformIconName(name ?? '', version ?? undefined); |
There was a problem hiding this comment.
Below lg we dropped the version before resolving the platform icon, but the icon is a fixed 20px glyph at every width. Since the version only picks between the legacy and modern Edge logos, this just meant Edge 12–18 rendered with the wrong logo on narrow screens
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 8f348e7. Configure here.
Replaces the two styled components with primitives. The responsive font-size now rides on Text's `size` prop, which emits the same @container rule. gray500 becomes the `secondary` content token it already mapped to, and text-align gives way to Stack's cross-axis centering. Refs #120315
The browser and OS columns only passed the version to generatePlatformIconName above a width threshold. The version selects between the legacy and modern Edge logos and is used nowhere else, so narrow layouts rendered legacy Edge (12-18) with the modern logo. The width check gave nothing back either way - the icon is a fixed 20px glyph and the tooltip already shows the version unconditionally. Refs #120315
8f348e7 to
f24a179
Compare
The title's container query never resolved - stories render outside any query container, so it was pinned to the 14px base instead of the 20px the pre-migration media query gave it. Make it a flat xl. Also nest the two text lines in their own Stack so the 8px gap applies between them and not below the illustration. Refs #120315
JoshuaKGoldberg
left a comment
JoshuaKGoldberg
left a comment
There was a problem hiding this comment.
I played around on the preview env and it looks great. And all the code removal is lovely. Thanks for this! 🙌
2 participants
Replay components now respond to the width of their containing surface instead of the viewport. Existing responsive thresholds are mapped to the nearest container-query tokens, and browser/OS icon selection uses the container-scoped responsive value hook.
The reduced-motion media query remains viewport-based because it represents a user media preference rather than available layout width.