Repository navigation
[HDX-5307] Top-align toolbars next to growing query fields #3151
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| '@hyperdx/app': patch | ||
| --- | ||
|
|
||
| fix: keep toolbar controls level with the first line of a wrapping query field | ||
|
|
||
| Date pickers, Run, and nearby row actions stay top-aligned as a SQL or Lucene field grows. Sessions opts into PageHeader block padding so a taller search bar still has room. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,6 +61,12 @@ export type PageHeaderProps = { | |
| * When omitted, the header behaves as a single sticky block. | ||
| */ | ||
| stickyRow?: React.ReactNode; | ||
| /** | ||
| * Add block padding that only shows once the toolbar outgrows `min-height`. | ||
| * Use for a header that can grow (Sessions' multi-line search); omit on | ||
| * title-only pages so they keep a flush 60px bar. | ||
| */ | ||
| growing?: boolean; | ||
|
Comment on lines
+64
to
+69
Contributor
There was a problem hiding this comment.
This user-facing Context Used: CLAUDE.md (source)
Member
Author
There was a problem hiding this comment. Added a patch changeset.
Contributor
There was a problem hiding this comment. 🔵 minor — New
Member
Author
There was a problem hiding this comment. Sessions already uses the |
||
| }; | ||
|
|
||
| export function PageHeader({ | ||
|
|
@@ -72,6 +78,7 @@ export function PageHeader({ | |
| className, | ||
| 'data-testid': testId, | ||
| stickyRow, | ||
| growing = false, | ||
| }: PageHeaderProps) { | ||
| const hasToolbar = title != null || leading != null || actions != null; | ||
| const hasBreadcrumbs = breadcrumbs != null; | ||
|
|
@@ -101,6 +108,12 @@ export function PageHeader({ | |
| // than the chrome `<header>`, which lets it stay pinned for the full | ||
| // length of the page rather than only the height of the header. | ||
| const chromeStickyClass = hasStickyRow ? styles.notSticky : undefined; | ||
| const chromeClassName = classNames( | ||
| styles.header, | ||
| growing && styles.growing, | ||
| chromeStickyClass, | ||
| className, | ||
| ); | ||
|
|
||
| const toolbarInner = ( | ||
| <> | ||
|
|
@@ -118,44 +131,28 @@ export function PageHeader({ | |
| // it. A page with only a `stickyRow` (no breadcrumbs, no title) | ||
| // skips the empty chrome entirely. | ||
| children != null ? ( | ||
| <header className={chromeClassName} data-testid={testId}> | ||
| {children} | ||
| </header> | ||
| ) : null | ||
| ) : hasBreadcrumbs && hasToolbar ? ( | ||
| <header | ||
| className={classNames(chromeClassName, styles.headerStacked)} | ||
| data-testid={testId} | ||
| > | ||
| <div className={styles.breadcrumbsRow}>{breadcrumbs}</div> | ||
| <div className={styles.toolbarRow}>{toolbarInner}</div> | ||
| </header> | ||
| ) : hasBreadcrumbs && !hasToolbar ? ( | ||
| <header | ||
| className={classNames(chromeClassName, styles.headerStacked)} | ||
| data-testid={testId} | ||
| > | ||
| <div className={styles.breadcrumbsRow}>{breadcrumbs}</div> | ||
| {children} | ||
| </header> | ||
| ) : ( | ||
| <header className={chromeClassName} data-testid={testId}> | ||
| {toolbarInner} | ||
| </header> | ||
| ); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔵 minor — The
growingprop and its:not(.notSticky)guard can be replaced by one unconditional line.headerisbox-sizing: border-boxwithmin-height: 60px, sopadding-block: var(--mantine-spacing-xs)(20px total) is absorbed for any content ≤40px tall — which covers every current non-stacked header: title-only pages,TeamPage.tsx:274(xs input, 30px),DashboardKioskHeader.tsx:14,DBServiceMapPage.tsx:343(Text/Slider/TimePicker, 36px). Applying it unconditionally to.headerlets.notSticky's laterpadding-block-end: 0win naturally, drops thegrowingprop, the:not(.notSticky)specificity workaround, and the doc edits inagent_docs/page_layout.mdand.claude/skills/page-layout/SKILL.md. It also removes a latent trap: as written,growing+stickyRowloses the top padding too, not just the bottom.Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond