fix(search): keep a constant gap between the search modal and the header - #1297
Draft
tmashara wants to merge 4 commits into
Draft
fix(search): keep a constant gap between the search modal and the header#1297tmashara wants to merge 4 commits into
tmashara wants to merge 4 commits into
Conversation
Mirrors metabase/metabase.github.io — both repos ship `/css/inkeep.css`, so the two files must stay identical. The Inkeep widget fixes its modal at `top: 5%` inside an overlay whose `backdrop-filter` makes it the containing block, so our `margin-top` override produced a gap that scaled with the viewport: 46px at 920px tall, 24px at 473px, and the panel ran 57px past the bottom of a short viewport. Anchor `top` to `--navigation-header-height` instead so the spacing is constant at every height, and clamp `max-height` so the results list scrolls rather than overflowing the screen. The mobile 85px header value flows through the same custom property. The `.ikp-modal__overlay` padding was dead code — the content is out of flow and positioned from the overlay's padding box, so it never contributed. Adds vitest to guard the offset, since the repo had no test runner. GRO-841 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ponent
Mirrors metabase/metabase.github.io. Markup, init script and shadow-DOM CSS now
live together in `src/components/search/`, inlined via
`theme.styles: [{ type: "style" }]` instead of a `/css/inkeep.css` fetched into
the shadow root — which also drops a render-blocking round trip, since cxkit
gates the whole widget on its linked sheets loading.
The offset arithmetic moves out of CSS into `header-offset.ts`, which measures
the header and publishes `--inkeep-modal-top`. This matters more here than on
marketing: `--navigation-header-height` is not defined in this repo at all, it
arrives from a stylesheet on the marketing origin, and the shared-chrome header
snapshot carries a live `.promo-banner`. Verified against a real build — with
the banner up the gap is 24px, and it stays 24px once the banner is dismissed.
The offset is scoped `:not([data-mobile])`. Below 768px the widget goes
full-screen through `inset: 0` at a specificity our `top` could not reach, while
our `max-height` still applied and stranded a dead strip at the bottom.
`learn/top-bar.html` becomes `TopBar.astro` so the widget can be a real
component rather than a liquid partial; it had a single consumer, and its
`domain != "docs"` and `show_search == "false"` branches were already dead.
Adds happy-dom and the render-astro harness so the component and its offset
logic are tested rather than the CSS text.
GRO-841
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…server Measured on a real build of this site: removing `.promo-banner` shrinks `.navigation-header` from 130px to 82px, and a ResizeObserver on either that element or the outer `header.bootstrap` does not fire at all. Only a childList mutation does. The banner is a direct child, so no subtree is needed. The ResizeObserver stays for the 77/85px breakpoint and font reflow. GRO-841 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tmashara
marked this pull request as draft
August 25, 2026 12:32
The header is sticky at `top: -48px` so the promo banner scrolls out while the nav row stays pinned: its bottom edge moves between 130px and 82px, and neither observer fires for it. Measured against a real build of this site, which has a live banner — opening the modal after scrolling put it 72px too low. Reads through on scroll rather than scheduling a frame: the browser already caps scroll events at one per frame, and requestAnimationFrame does not run in a hidden tab. GRO-841 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirrors metabase/metabase.github.io#6422 — same architecture.
Inkeep fixes its modal at
top: 5%, so the gap under our header scaled with the viewport — 46px at 920px tall, 24px at 473px, and the panel ran 57px off the bottom of a short viewport.Extracts the widget into
src/components/search/— markup, init script and shadow CSS together, inlined viatheme.styles: [{ type: "style" }]instead of a/css/inkeep.cssfetched into the shadow root.learn/top-bar.htmlbecomesTopBar.astroso the widget can be a real component rather than a liquid partial; it had a single consumer and itsdomain != "docs"/show_search == "false"branches were already dead.The offset arithmetic moves out of CSS into
header-offset.ts, which measures the header and publishes--inkeep-modal-top. This matters more here than on marketing:--navigation-header-heightis not defined in this repo at all — it arrives from a stylesheet on the marketing origin — and the shared-chrome header snapshot carries a live.promo-banner.Verified against a real build of this site: with the banner up the gap is 24px, and it stays 24px once the banner is dismissed. Also measured there that a
ResizeObserverdoes not fire when the banner is removed — only achildListmutation does — and there's a test that fails if that observer is dropped.The offset is scoped
:not([data-mobile]): below 768px the widget goes full-screen viainset: 0at a specificity ourtopcould not reach, while ourmax-heightstill applied and stranded a dead strip at the bottom.Adds happy-dom and the render-astro harness so the component and its offset logic are tested rather than the CSS text.
GRO-841 · Slack thread
🤖 Generated with Claude Code