ref(layout) align page paddings#111823
ref(layout) align page paddings#111823JonasBa wants to merge 6 commits intojb/pageframe/layoutpagefrom
Conversation
github-actions
bot
added
the
Scope: Frontend
| primaryNavigation.layout === 'sidebar' && | ||
| secondaryNavigation?.view === 'expanded' | ||
| } | ||
| padding={props.withPadding ? '2xl 3xl' : undefined} |
There was a problem hiding this comment.
We never pass this for page frame components so it can be removed
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| borderTop={ | ||
| hasPageFrame ? undefined : background === 'secondary' ? 'primary' : undefined | ||
| } | ||
| borderTop={background === 'secondary' ? 'primary' : undefined} |
There was a problem hiding this comment.
Removed hasPageFrame guard causes unwanted border
Medium Severity
The hasPageFrame guard was removed from the borderTop condition. Previously, when hasPageFrame was true, borderTop was always undefined. Now it evaluates background === 'secondary' ? 'primary' : undefined regardless of hasPageFrame. Since every caller passes background="secondary", this introduces an unwanted borderTop in the page-frame layout that was previously explicitly suppressed.
… layout
The non-pageFrame padding should use {xs: 'xl', md: '3xl'} to match the old CSS behavior:
- Screens < 992px: xl padding
- Screens >= 992px: 3xl padding
Previously used {md: 'xl', lg: '3xl'} which incorrectly applied xl padding to screens between 992-1200px.
Co-authored-by: Jonas <JonasBa@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| import {useExplorerPanel} from 'sentry/views/seerExplorer/useExplorerPanel'; | ||
| import {isSeerExplorerEnabled} from 'sentry/views/seerExplorer/utils'; | ||
|
|
||
| import {NAVIGATION_MOBILE_TOPBAR_HEIGHT_WITH_PAGE_FRAME} from './constants'; |
There was a problem hiding this comment.
Duplicate imports from same constants module
Low Severity
Two separate import statements pull from './constants' — NAVIGATION_MOBILE_TOPBAR_HEIGHT_WITH_PAGE_FRAME and PRIMARY_HEADER_HEIGHT. These can be combined into a single import statement.
2 participants
Audit pages for layout use so that menus, headers and page contents are aligned horizontally with each other. Since many layouts use bespoke styling, this is only the first pass of pages that are easily visible by going through the list of some top level routes