Skip to content

feat(dashboard): migrate to PageLayout with sticky query toolbar#2364

Merged
kodiakhq[bot] merged 3 commits into
mainfrom
page-migrate-dashboard
Jun 1, 2026
Merged

feat(dashboard): migrate to PageLayout with sticky query toolbar#2364
kodiakhq[bot] merged 3 commits into
mainfrom
page-migrate-dashboard

Conversation

@elizabetdev

Copy link
Copy Markdown
Contributor

Summary

Migrates the Dashboard page to the shared PageLayout / PageHeader shell and introduces a reusable stickyRow slot on PageHeader so any page can pin a single row while the rest of the header scrolls away.

On the dashboard:

  • Sticky row: the query toolbar (SQL/Lucene WHERE, time range, granularity, Live, refresh, edit filters, Run) stays pinned at the top of the scroll container while the user scrolls through tiles.
  • Scrolling chrome: breadcrumbs, the editable dashboard name, dashboard actions (Favorite / Tags / Menu) and the "Created by … Updated …" meta sit above and scroll away with the page.
  • Meta moves into the breadcrumbs row (right side) instead of taking its own body line.
  • Redundant mt="sm" removed from DashboardFiltersPageLayout's padded content already provides the top offset.

The stickyRow slot

<PageHeader
  breadcrumbs={pageBreadcrumbs}    // scrolls away
  stickyRow={queryToolbar}          // pinned to top — the only sticky row
>
  {titleRow}                         // scrolls away
</PageHeader>

Behavior:

  • When stickyRow is provided, the chrome <header> becomes non-sticky and drops its bottom border / bottom padding so it reads as one block with the toolbar at rest.
  • PageHeader returns a Fragment of <header> + sticky <div> so the sticky row's containing block is the page layout root — it stays pinned for the full scroll length rather than being bounded by the chrome's height (the classic position: sticky gotcha).
  • An IntersectionObserver tracks when the row touches the scroll-container top and toggles a .stickyRowAttached class so the row has no padding-top while attached to chrome (tight at rest) and standard padding once stuck (breathing room from the viewport edge). Also exposes data-stuck on the row for tests / debugging.
  • When stickyRow is omitted, PageHeader behaves exactly like before — single sticky block. No other page needs to change.

Test plan

  • yarn ci:unit in packages/app still passes.
  • Open a saved dashboard. Confirm:
    • At rest: chrome (breadcrumbs + name + actions) and toolbar read as one continuous block, no gap and no extra horizontal line between them.
    • Scrolling down: chrome scrolls away; the query toolbar stays pinned at the top of the scroll area with comfortable top padding.
    • Scrolling back up: chrome reappears, toolbar tucks back into "attached" mode (top padding removed).
    • "Created by … Updated …" meta sits on the right side of the breadcrumbs row with the existing tooltip preserved.
  • Open any non-dashboard page that uses PageHeader (Alerts list, Search page, Sessions, Service Map, Kubernetes). Confirm the header still behaves identically to main (fully sticky, no visual regression).
  • Open a dashboard with no defined dashboard variables — DashboardFilters row still sits at the right vertical offset (no doubled top padding).

Made with Cursor

Replaces the custom Dashboard page chrome with the shared
`PageLayout` / `PageHeader` shell and introduces a reusable
`stickyRow` slot on `PageHeader` so any page can pin a single row
while the rest of the header scrolls away.

On the dashboard this is used to keep the query toolbar (SQL/Lucene
WHERE, time range, granularity, Live, refresh, edit filters, Run)
visible while the user scrolls through tiles. The breadcrumbs, the
editable dashboard name, dashboard actions (Favorite, Tags, Menu) and
the "Created by … Updated …" meta all live in the scrolling chrome
above; the meta moves to the right edge of the breadcrumbs row
instead of taking its own body line.

PageHeader changes:
- New `stickyRow?: ReactNode` slot. When provided, the chrome
  `<header>` becomes non-sticky (and drops its bottom border /
  bottom padding so it reads as one block with the toolbar), and a
  sibling `<div>` renders the `stickyRow` at `position: sticky;
  top: 0`. Returning a Fragment of the two siblings means the
  sticky row's containing block is the page layout root, so it
  stays pinned for the full scroll length rather than being bounded
  by the chrome height.
- `IntersectionObserver` tracks when the sticky row touches the
  scroll-container top and toggles a `.stickyRowAttached` class so
  the row has no `padding-top` while attached to chrome (tight
  spacing at rest) and the standard `--mantine-spacing-sm` top
  padding once stuck (breathing room from the viewport edge). The
  state is also exposed as `data-stuck` on the row for tests.
- Pages without a `stickyRow` keep the existing fully-sticky header
  behavior — no migration needed elsewhere.

Also drops a redundant `mt="sm"` from `DashboardFilters`, since the
`PageLayout` `padded` content already provides top padding above it.

Co-authored-by: Cursor <cursoragent@cursor.com>
@vercel

vercel Bot commented May 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment Jun 1, 2026 11:48am
hyperdx-storybook Ready Ready Preview, Comment Jun 1, 2026 11:48am

Request Review

@changeset-bot

changeset-bot Bot commented May 28, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 00ddc83

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch
@hyperdx/otel-collector Patch

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

@github-actions github-actions Bot added the review/tier-3 Standard — full human review required label May 28, 2026
@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

🟡 Tier 3 — Standard

Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.

Why this tier:

  • Diff size: 808 production lines changed (Tier 2 max: < 250)

Review process: Full human review — logic, architecture, edge cases.
SLA: First-pass feedback within 1 business day.

Stats
  • Production files changed: 4
  • Production lines changed: 808
  • Branch: page-migrate-dashboard
  • Author: elizabetdev

To override this classification, remove the review/tier-3 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 191 passed • 3 skipped • 1323s

Status Count
✅ Passed 191
❌ Failed 0
⚠️ Flaky 4
⏭️ Skipped 3

Tests ran across 4 shards in parallel.

View full report →

@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found.

🟡 P2 -- recommended

  • packages/app/src/components/PageHeader.tsx:91 -- The new IntersectionObserver defaults root to the viewport, but position: sticky on .stickyRow resolves against #app-content-scroll-container (packages/app/src/layout.tsx:63); when the ClickStack banner is open the scroll-container top is offset below the viewport top, so the row never crosses the -1px rootMargin and isStuck stays false, leaving .stickyRowAttached (padding-top: 0) applied permanently in the stuck state.
    • Fix: Resolve the scroll container inside the effect (e.g. root: document.getElementById('app-content-scroll-container')) so the observer measures intersection against the same containing block CSS sticky uses.
    • kieran-typescript, julik-frontend-races, correctness
  • packages/app/src/components/PageHeader.tsx:86 -- The new stickyRow slot adds four header-shape branches, a chromeStickyClass toggle, an IntersectionObserver-driven isStuck state, a .stickyRowAttached padding swap, and a data-stuck attribute -- none of which have any unit test, snapshot test, or Playwright assertion covering the new behavior.
    • Fix: Add packages/app/src/components/__tests__/PageHeader.test.tsx that exercises each variant (toolbar-only, breadcrumbs-only, breadcrumbs+toolbar, children-only, and the new chrome+stickyRow and stickyRow-only branches) and drives a mocked IntersectionObserver to assert the data-stuck and .stickyRowAttached toggle.
    • testing, julik-frontend-races, maintainability
  • packages/app/src/DBDashboardPage.tsx:2253 -- The new <Flex justify="space-between" align="center" gap="sm" w="100%"> carrying dashboardMeta with flexShrink: 0 replaces the previous <Group align="flex-start" justify="space-between">, whose default wrap="wrap" let the meta drop below on narrow viewports; long createdBy emails now squash the breadcrumb instead of wrapping.
    • Fix: Either set wrap="wrap" on the outer Flex (matching prior behavior) or drop flexShrink: 0 from dashboardMeta and add truncation semantics so it can shrink.
    • adversarial, correctness
🔵 P3 nitpicks (9)
  • packages/app/src/components/PageHeader.tsx:115 -- The four header-shape branches are now folded into a single nested ternary const chrome = ..., harder to scan than the previous four labelled early returns.
    • Fix: Extract a renderChrome() helper with named early-return branches and let the outer return decide whether to wrap in a Fragment with the sticky row.
    • maintainability, kieran-typescript
  • packages/app/src/components/PageHeader.tsx:174 -- chrome != null && !isStuck && styles.stickyRowAttached mixes a structural fact (whether chrome rendered this pass) with a runtime state in one inline guard.
    • Fix: Hoist into a named boolean such as const isPinnedDirectlyUnderChrome = chrome != null && !isStuck; and use that in classNames.
    • maintainability, kieran-typescript
  • packages/app/src/components/PageHeader.module.scss:25 -- .notSticky is an inverter class whose only job is to revert five .header declarations, coupling the two rules.
    • Fix: Split .header into a layout base plus a .headerSticky modifier and apply the modifier only when no stickyRow is provided.
    • maintainability
  • packages/app/src/components/PageLayout.tsx:27 -- PageLayoutProps's Pick<PageHeaderProps, ...> omits stickyRow, so structured-slot callers can't get a sticky toolbar without dropping to the header={<PageHeader stickyRow={...}/>} escape hatch.
    • Fix: Add 'stickyRow' to the Pick set and forward it through the internal <PageHeader> instantiation, or document the escape hatch in PageLayout's JSDoc.
    • kieran-typescript, api-contract
  • packages/app/src/components/PageHeader.tsx:115 -- A <PageHeader /> with no props/children now returns null instead of an empty <header>; no in-tree caller hits this today but it is an undocumented contract change.
    • Fix: Either document the empty-no-props null return on PageHeaderProps, or keep the empty <header> for symmetry.
    • api-contract, adversarial
  • packages/app/src/components/PageHeader.tsx:91 -- The IO callback destructures ([entry]) with no defensive read; valid for a single observed node but brittle to future refactors.
    • Fix: Use entries[0]?.intersectionRatio ?? 1 or otherwise null-check before reading.
    • kieran-typescript
  • packages/app/src/components/PageHeader.tsx:86 -- isStuck initializes to false, so on bfcache restore or deep-scroll mount the row paints .stickyRowAttached for one frame before IO posts the corrected state.
    • Fix: Compute initial stuck state in a useLayoutEffect from node.getBoundingClientRect().top so the first paint is correct.
    • julik-frontend-races, adversarial
  • packages/app/src/setupTests.tsx:14 -- setupTests.tsx polyfills ResizeObserver and matchMedia but not IntersectionObserver; any future jsdom test that renders PageHeader with stickyRow will throw ReferenceError: IntersectionObserver is not defined.
    • Fix: Add a class stub for IntersectionObserver mirroring the existing ResizeObserver shim.
    • julik-frontend-races
  • agent_docs/page_layout.md:31 -- The PageHeader API table is not updated for the new stickyRow prop, and the "Pages using PageHeader / PageLayout today" list does not include Dashboards.
    • Fix: Add a stickyRow row to the API table with a brief usage note, and add Dashboards to the consumers list.
    • project-standards

Reviewers (8): correctness, testing, maintainability, project-standards, kieran-typescript, julik-frontend-races, adversarial, api-contract.

Testing gaps:

  • No unit test exists for PageHeader.tsx (any variant), PageLayout.tsx, or DBDashboardPage.tsx; the new stickyRow, isStuck, .stickyRowAttached, notSticky, and data-stuck behaviors are uncovered.
  • No Playwright assertion covers the ClickStack-build-with-banner-open shell where the IO root mismatch surfaces.
  • No regression test asserts that the four pre-existing PageHeader chrome shapes (used by AlertsPage, DashboardsListPage, SavedSearchesListPage, SessionsPage, TeamPage) render unchanged when stickyRow is omitted.

@kodiakhq kodiakhq Bot merged commit 538a1c4 into main Jun 1, 2026
19 checks passed
@kodiakhq kodiakhq Bot deleted the page-migrate-dashboard branch June 1, 2026 11:54
brandon-pereira pushed a commit that referenced this pull request Jun 2, 2026
## Summary

Migrates the Dashboard page to the shared `PageLayout` / `PageHeader` shell and introduces a reusable `stickyRow` slot on `PageHeader` so any page can pin a single row while the rest of the header scrolls away.

On the dashboard:
- **Sticky row**: the query toolbar (SQL/Lucene WHERE, time range, granularity, Live, refresh, edit filters, Run) stays pinned at the top of the scroll container while the user scrolls through tiles.
- **Scrolling chrome**: breadcrumbs, the editable dashboard name, dashboard actions (Favorite / Tags / Menu) and the "Created by … Updated …" meta sit above and scroll away with the page.
- **Meta moves into the breadcrumbs row** (right side) instead of taking its own body line.
- **Redundant `mt="sm"`** removed from `DashboardFilters` — `PageLayout`'s `padded` content already provides the top offset.

## The `stickyRow` slot

```tsx
<PageHeader
  breadcrumbs={pageBreadcrumbs}    // scrolls away
  stickyRow={queryToolbar}          // pinned to top — the only sticky row
>
  {titleRow}                         // scrolls away
</PageHeader>
```

Behavior:
- When `stickyRow` is provided, the chrome `<header>` becomes non-sticky and drops its bottom border / bottom padding so it reads as one block with the toolbar at rest.
- `PageHeader` returns a Fragment of `<header>` + sticky `<div>` so the sticky row's containing block is the page layout root — it stays pinned for the full scroll length rather than being bounded by the chrome's height (the classic `position: sticky` gotcha).
- An `IntersectionObserver` tracks when the row touches the scroll-container top and toggles a `.stickyRowAttached` class so the row has no `padding-top` while attached to chrome (tight at rest) and standard padding once stuck (breathing room from the viewport edge). Also exposes `data-stuck` on the row for tests / debugging.
- When `stickyRow` is omitted, `PageHeader` behaves exactly like before — single sticky block. **No other page needs to change.**

## Test plan

- [ ] `yarn ci:unit` in `packages/app` still passes.
- [ ] Open a saved dashboard. Confirm:
  - At rest: chrome (breadcrumbs + name + actions) and toolbar read as one continuous block, no gap and no extra horizontal line between them.
  - Scrolling down: chrome scrolls away; the query toolbar stays pinned at the top of the scroll area with comfortable top padding.
  - Scrolling back up: chrome reappears, toolbar tucks back into "attached" mode (top padding removed).
  - "Created by … Updated …" meta sits on the right side of the breadcrumbs row with the existing tooltip preserved.
- [ ] Open any non-dashboard page that uses `PageHeader` (Alerts list, Search page, Sessions, Service Map, Kubernetes). Confirm the header still behaves identically to `main` (fully sticky, no visual regression).
- [ ] Open a dashboard with no defined dashboard variables — `DashboardFilters` row still sits at the right vertical offset (no doubled top padding).

Made with [Cursor](https://cursor.com)
jordan-simonovski pushed a commit that referenced this pull request Jun 3, 2026
Migrates the Dashboard page to the shared `PageLayout` / `PageHeader` shell and introduces a reusable `stickyRow` slot on `PageHeader` so any page can pin a single row while the rest of the header scrolls away.

On the dashboard:
- **Sticky row**: the query toolbar (SQL/Lucene WHERE, time range, granularity, Live, refresh, edit filters, Run) stays pinned at the top of the scroll container while the user scrolls through tiles.
- **Scrolling chrome**: breadcrumbs, the editable dashboard name, dashboard actions (Favorite / Tags / Menu) and the "Created by … Updated …" meta sit above and scroll away with the page.
- **Meta moves into the breadcrumbs row** (right side) instead of taking its own body line.
- **Redundant `mt="sm"`** removed from `DashboardFilters` — `PageLayout`'s `padded` content already provides the top offset.

```tsx
<PageHeader
  breadcrumbs={pageBreadcrumbs}    // scrolls away
  stickyRow={queryToolbar}          // pinned to top — the only sticky row
>
  {titleRow}                         // scrolls away
</PageHeader>
```

Behavior:
- When `stickyRow` is provided, the chrome `<header>` becomes non-sticky and drops its bottom border / bottom padding so it reads as one block with the toolbar at rest.
- `PageHeader` returns a Fragment of `<header>` + sticky `<div>` so the sticky row's containing block is the page layout root — it stays pinned for the full scroll length rather than being bounded by the chrome's height (the classic `position: sticky` gotcha).
- An `IntersectionObserver` tracks when the row touches the scroll-container top and toggles a `.stickyRowAttached` class so the row has no `padding-top` while attached to chrome (tight at rest) and standard padding once stuck (breathing room from the viewport edge). Also exposes `data-stuck` on the row for tests / debugging.
- When `stickyRow` is omitted, `PageHeader` behaves exactly like before — single sticky block. **No other page needs to change.**

- [ ] `yarn ci:unit` in `packages/app` still passes.
- [ ] Open a saved dashboard. Confirm:
  - At rest: chrome (breadcrumbs + name + actions) and toolbar read as one continuous block, no gap and no extra horizontal line between them.
  - Scrolling down: chrome scrolls away; the query toolbar stays pinned at the top of the scroll area with comfortable top padding.
  - Scrolling back up: chrome reappears, toolbar tucks back into "attached" mode (top padding removed).
  - "Created by … Updated …" meta sits on the right side of the breadcrumbs row with the existing tooltip preserved.
- [ ] Open any non-dashboard page that uses `PageHeader` (Alerts list, Search page, Sessions, Service Map, Kubernetes). Confirm the header still behaves identically to `main` (fully sticky, no visual regression).
- [ ] Open a dashboard with no defined dashboard variables — `DashboardFilters` row still sits at the right vertical offset (no doubled top padding).

Made with [Cursor](https://cursor.com)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge review/tier-3 Standard — full human review required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants