Migrate Kubernetes dashboard to PageLayout#2347
Conversation
🦋 Changeset detectedLatest commit: 0f145fe The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🔵 Tier 2 — Low RiskSmall, isolated change with no API route or data model modifications. Why this tier:
Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns. Stats
|
Deep ReviewThis is a small, contained UI refactor: 🟡 P2 -- recommended
🔵 P3 nitpicks (2)
Reviewers (6): correctness, testing, maintainability, project-standards, kieran-typescript, adversarial. Testing gaps:
|
E2E Test Results✅ All tests passed • 191 passed • 3 skipped • 1287s
Tests ran across 4 shards in parallel. |
Sticky header includes breadcrumbs, log/metric sources, and time controls without a duplicate page title. Co-authored-by: Cursor <cursoragent@cursor.com>
elizabetdev
force-pushed
the
page-migrate-kubernetes
branch
from
2bca3e2 to
1a155c0
Compare
The PageLayout migration intentionally dropped the visible
`<Text size="xl">Kubernetes Dashboard</Text>` heading — the
`Dashboards / Kubernetes` breadcrumb is the new identifier
(see `PageHeader` docs: "omit `title` when the toolbar has
inputs"). The remaining occurrence of "Kubernetes Dashboard"
lives inside `<Head><title>`, which `page.getByText` cannot match.
That left the `should load kubernetes dashboard` smoke test
asserting on an empty locator. Update the page object to follow
the existing convention — every other page object exposes
`pageContainer = page.getByTestId('<page>-page')` (see
`SavedSearchesListPage`, `DashboardsListPage`, `TeamPage`,
`ClickHouseDashboardPage`, etc.) — and assert on that testid,
which `<PageLayout>` already renders on the page root.
Also drop the now-unused `dashboardTitle` field and `title`
getter so the page object's surface matches its actual locators.
…ageHeader composition forms
Two follow-up cleanups on the PageLayout migration:
P2 — Extract `dashboardBody`.
The ~380-line inline `content={<>...</>}` fragment in
`KubernetesDashboardPage` added three indentation levels to the
already-deeply-nested tab / chart JSX, forcing the
`convertV1ChartConfigToV2({...})` config objects to wrap further than
necessary. The author already extracted `headerLeading`, `headerActions`,
and `pageBreadcrumbs` into local consts; the body deserved the same
treatment. The render now reads as a small `return (<PageLayout ...
content={dashboardBody} />)` with all four header / body fragments
declared above. Prettier reformatted the body to the new indentation;
the diff looks larger than the semantic change.
P3 — Document the two valid composition forms.
`PageLayout` accepts both structured slots (`title` / `leading` /
`actions` / `breadcrumbs`, used by `KubernetesDashboardPage`) and a
custom `header` prop (used by `SessionsPage` for its full-width
source / search / time / Run row, which doesn't split cleanly into
leading + actions). Both are valid, but with only two consumers so
far it's worth picking a canonical form before more pages migrate.
Strengthened the JSDoc on `PageLayoutProps`, `PageHeaderProps`,
`children`, and `header` to:
- spell out the two forms in order of preference (structured slots
first, custom `header` / `children` as the escape hatch),
- cite the concrete pages that exemplify each form,
- explain the criterion for reaching for the escape hatch (the
toolbar can't be expressed as leading + actions, e.g. a single
full-width Group with a search input that grows to 50% width).
No code-level changes to `SessionsPage` — its layout genuinely doesn't
fit the structured slots, so forcing a refactor would be worse than
the documented escape hatch.
github-actions
Bot
added
review/tier-2
Main now contains two additional structured-slot users: - AlertsPage / DashboardsListPage / SavedSearchesListPage (#2345) now use `<PageHeader title="..." />` instead of the children form. - DBServiceMapPage (#2346) uses `<PageLayout leading actions />`. Update PageLayout / PageHeader JSDoc to cite the three concrete slot shapes (title-only, leading+actions, breadcrumbs+leading+actions) with the now-canonical examples, so future page migrations have an obvious reference for which form to pick.
2 participants
Summary
PageLayoutwith breadcrumbs and controls in the sticky header (log + metric sources, time range, refresh).Kubernetes Dashboard – {brandName}) that was added in feat: Minor dashboard improvements #2302.Recreated from #2285 now that the base
PageLayout/PageHeaderwork has landed onmain. The cherry-pick conflicted with #2302 (which added the<Head>tab title); resolution keeps the new tab title inside thePageLayoutcontent.Test plan
Made with Cursor