Skip to content

ref(layout): use Layout.Page on isolated pages#111644

Merged
priscilawebdev merged 9 commits intomasterfrom
JonasBa/ref/layout-page-isolated-pages
Mar 27, 2026
Merged

ref(layout): use Layout.Page on isolated pages#111644
priscilawebdev merged 9 commits intomasterfrom
JonasBa/ref/layout-page-isolated-pages

Conversation

@JonasBa
Copy link
Copy Markdown
Member

@JonasBa JonasBa commented Mar 26, 2026

Wraps non-compliant routes in isolated page areas with Layout.Page.
Part of the Layout.Page audit remediation.

Changes

  • static/app/views/insights/pages/conversations/layout.tsx: moved Layout.Page from overview.tsx to the layout component so it wraps both the header and outlet content
  • static/app/views/insights/pages/conversations/overview.tsx: removed Layout.Page (now handled by layout)
  • static/app/views/profiling/continuousProfileProvider.tsx: added Layout.Page wrapping header + outlet so flamegraph pages get correct framing
  • static/app/views/profiling/transactionProfileProvider.tsx: added Layout.Page wrapping header + outlet so flamechart pages get correct framing
  • static/app/views/profiling/continuousProfileFlamegraph.tsx: removed Layout.Page (now handled by provider)
  • static/app/views/profiling/profileFlamechart.tsx: removed Layout.Page (now handled by provider)
  • static/app/views/profiling/profileSummary/index.tsx: wrap ProfileSummaryPageToggle with Layout.Page
  • static/app/views/profiling/differentialFlamegraph.tsx: wrap DifferentialFlamegraphWithProviders with Layout.Page
  • static/app/views/projectInstall/newProject.tsx: wrap NewProject content with Layout.Page

Affected routes

Path Strategy
/organizations/:orgId/explore/conversations/ layout-level wrap (header + outlet)
/organizations/:orgId/profiling/profile/:projectId/flamegraph/ provider-level wrap (header + outlet)
/organizations/:orgId/profiling/profile/:projectId/:eventId/flamegraph/ provider-level wrap (header + outlet)
/organizations/:orgId/profiling/summary/:projectId/ leaf fix
/organizations/:orgId/profiling/profile/:projectId/differential-flamegraph/ leaf fix
/organizations/:orgId/projects/new/ leaf fix

Verification

  • Each route above loads in the browser and renders inside a <main> element

Wraps non-compliant routes in isolated page areas with Layout.Page.
Part of the Layout.Page audit remediation.
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 26, 2026
Move Layout.Page from the overview page component to the layout
component so it wraps both the header and the outlet. This ensures
Layout.Header is a direct child of Layout.Page, giving it the
correct border-radius and framed appearance.
Move Layout.Page from individual profiling pages to the shared
ProfilingContainer so all sub-pages (including flamegraph) get
the correct framing.
Profiling sub-pages individually manage Layout.Page (some with
custom styling like hiding the footer), so the container-level
approach causes double-wrapping. Restore Layout.Page on individual
pages instead.
Move Layout.Page from flamegraph child pages to the provider
components (continuousProfileProvider, transactionProfileProvider)
so it wraps both the header and the outlet content.
@priscilawebdev priscilawebdev marked this pull request as ready for review March 27, 2026 09:43
@priscilawebdev priscilawebdev requested review from a team as code owners March 27, 2026 09:43
Copy link
Copy Markdown
Member

@priscilawebdev priscilawebdev left a comment

Choose a reason for hiding this comment

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

manually checked the changes and it looks good to me.

Comment thread static/app/views/profiling/differentialFlamegraph.tsx Outdated
Comment thread static/app/views/profiling/profileSummary/index.tsx Outdated
) : null}
<Outlet />
</Fragment>
</Layout.Page>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Layout wrapper at wrong level causes nested main elements

Medium Severity

Layout.Page is added to ConversationsLayout, which wraps all child routes via Outlet — including transactionSummaryRoute and traceView, both of which render their own Layout.Page. This creates nested <main> elements (semantically invalid HTML) and breaks the ~ footer { display: none; } CSS sibling selector on traceView's inner LayoutPageWithHiddenFooter, since the footer is now a sibling of the outer <main> only. The PR description indicates the fix belongs in overview.tsx, not layout.tsx.

Fix in Cursor Fix in Web

Copy link
Copy Markdown
Member

@priscilawebdev priscilawebdev Mar 27, 2026

Choose a reason for hiding this comment

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

Not a bug. The conversations page renders a table and drawer. transactionSummaryRoute and traceView are included as children following the same pattern as all other insight pages (frontend, backend, mobile, etc.). No visual or functional issue observed.

priscilawebdev and others added 2 commits March 27, 2026 11:37
Move footer-hiding CSS from ProfileSummaryContainer to the Layout.Page
wrapper. The ~ footer sibling selector broke when Layout.Page was added
as a parent, since the footer is now a sibling of <main>, not the inner
container.

Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
Same fix as profile summary — move footer-hiding CSS from the inner
container to the Layout.Page wrapper so the sibling selector still
matches the footer element.

Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment thread static/app/views/profiling/continuousProfileProvider.tsx Outdated
Deduplicate the identical styled component from four profiling files
into a single export in profiling/utils.tsx.

Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
Move from utils.tsx to its own component file for clarity.

Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
@priscilawebdev priscilawebdev merged commit 9bcfd0f into master Mar 27, 2026
69 of 70 checks passed
@priscilawebdev priscilawebdev deleted the JonasBa/ref/layout-page-isolated-pages branch March 27, 2026 11:03
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants