fix(announcements): float the banner instead of occupying layout - #979
Merged
Conversation
Dismissing the banner pulled the whole view up by its height. It was a flex child of the shell's `<main>`, so appearing and disappearing reflowed everything below it — the jump was the bug, and reserving the space forever would have been a worse fix. It is now positioned `absolute` against a `relative` `<main>`: a rounded, shadowed pill floating over the content rather than a full-bleed strip displacing it. Measured before and after a dismissal, every content element — scroll container, greeting, composer — moves by exactly 0px in both axes. The overlay removes the reason anything had to know the banner's size, so this deletes more than it adds: - `--announcement-banner-height`, its `ResizeObserver`, the height signal, and the `DOCUMENT`/`ElementRef`/`DestroyRef` injections all go - `.chat-topnav-wrapper` goes back to `top: 0` - `.chat-container-empty.full-page` goes back to `inset: 0` - both floating sidenav control clusters go back to `top-4` `top-16` is the one constant that replaces all of it, and it is not arbitrary. On a chat route it lands the pill immediately below the fixed topnav — the placement §D1 asks for — and everywhere else it clears the shell's floating sidebar buttons, which sit at `top-4` and would otherwise be overlapped by a centred pill on any viewport narrow enough for the two to meet. Verified at 375px: the controls end at y=56 and the pill starts at y=64. The positioning strip spans the full content width, so it is `pointer-events-none` with `pointer-events-auto` on the pill alone — otherwise an invisible band would swallow clicks aimed at the topnav and the sidebar buttons beneath it. Verified: a click 30px outside the pill lands on the chat container, not the banner. `relative` on `<main>` is load-bearing. Without it the pill anchors to the viewport and drifts out from under the sidenav's padding transition. Browser-verified against dev data in light and dark and at 375px, with no horizontal overflow. Full frontend suite 2474 passed. 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
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.
Follow-up to #976. Dismissing the banner pulled the whole view up by its height, because it was a flex child of the shell's
<main>— appearing and disappearing reflowed everything below it. Reserving the space forever would have been a worse fix.It is now positioned
absoluteagainst arelative<main>: a rounded, shadowed pill floating over the content rather than a full-bleed strip displacing it.Measured before and after a dismissal, every content element moves by exactly 0px in both axes:
#app-scroll-containerIt deletes more than it adds
Once the banner stops occupying space, nothing needs to know its size — so the whole measurement mechanism and all three offsets from #976 come out:
--announcement-banner-height, itsResizeObserver, the height signal, and theDOCUMENT/ElementRef/DestroyRefinjections.chat-topnav-wrapper→ back totop: 0.chat-container-empty.full-page→ back toinset: 0top-4Net −111/+83 lines.
Two details worth reviewing
top-16is the single constant that replaces all of it, and it is not arbitrary. On a chat route it lands the pill immediately below the fixed topnav — the placement §D1 asks for — and everywhere else it clears the shell's floating sidebar buttons attop-4, which a centred pill would otherwise overlap on any viewport narrow enough for the two to meet. Verified at 375px: controls end at y=56, pill starts at y=64,overlapsControls: false. One constant, no route awareness.The positioning strip spans the full content width, so it is
pointer-events-nonewithpointer-events-autoon the pill alone. Without that, an invisible band would swallow every click aimed at the topnav and the sidebar buttons underneath it. Verified: a click 30px outside the pill resolves to.chat-container-empty, not the banner.relativeon<main>is load-bearing — without it the pill anchors to the viewport and drifts out from under the sidenav's padding transition.Verification
Browser-verified against real dev data with a local app-api: light and dark, desktop and 375px, no horizontal overflow,
hostLeft: 288confirming it anchors to the content region rather than the viewport, and the dismiss still writing its durable server-side ack.Specs updated in place — the two height-publishing tests are replaced by five overlay ones covering absolute positioning, click pass-through, the
top-16clearance, the rounded-card styling, and the absence of any document-level layout variable. Full frontend suite 2474 passed.Test data created in dev was cleaned up; only real users' acks and the existing "Welcome to Dev" announcement remain.
Independent of #978 (PR-6) — no shared files, checked rather than assumed.
🤖 Generated with Claude Code