chore: improve scroll behavior of DashboardLayout wrapped pages #19396
Conversation
There was a problem hiding this comment.
lgtm! this is a precarious enough change I'd like to get a second pair of eyes on it tho, maybe @Parkreiner?
|
Agreed, and if someone has time to pull this down and check it out locally I'd appreciate it too. A weak point of our storybook setup is we don't really test for scrolling content as often as we should |
|
I ran it locally and didn't notice any jank, I'm just paranoid lol |
| @@ -76,7 +76,7 @@ const GroupsPage: FC = () => { | |||
| } | |||
|
|
|||
| return ( | |||
| <> | |||
| <div className="w-full max-w-screen-2xl pb-10"> | |||
There was a problem hiding this comment.
Is there any way we can refactor the layouts to bake the pb-10 in by default? I'm worried that it could get missed as we add new page types in the future
There was a problem hiding this comment.
I believe I tried doing that, but the pages aren't set up consistently so it didn't end up being feasible. And making the pages more consistent seemed out of scope for this PR
| @@ -108,7 +108,7 @@ export const TemplateLayout: FC<PropsWithChildren> = ({ | |||
|
|
|||
| if (error || workspacePermissionsQuery.error) { | |||
| return ( | |||
| <div css={{ margin: 16 }}> | |||
| <div className="m-4"> | |||
There was a problem hiding this comment.
Totally okay if this isn't feasible because it would require too many changes elsewhere, but could we update this to use padding instead? I feel like margins are an anti-pattern ~95% of the time
Updates the the
DashboardLayoutto create a singular scroll area between the top nav bar and the deployment banner on the bottom. Also improves the scroll behavior of the org settings pages.CleanShot.2025-08-18.at.13.49.29.mp4