Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 7 additions & 12 deletions frontend/ai.client/src/app/app.html
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@

<!-- Desktop controls (when sidenav collapsed) -->
@if (sidenavService.isCollapsed() && !chromeHidden()) {
<div
class="hidden lg:flex fixed left-4 z-50 gap-2"
[style.top]="'calc(1rem + var(--announcement-banner-height, 0px))'">
<div class="hidden lg:flex fixed top-4 left-4 z-50 gap-2">
<!-- Expand sidebar button -->
<button
type="button"
Expand Down Expand Up @@ -74,9 +72,7 @@

<!-- Mobile controls (when header content hidden and sidenav not visible) -->
@if (!headerService.showContent() && !sidenavService.isVisible() && !chromeHidden()) {
<div
class="flex lg:hidden fixed left-4 z-50 gap-2"
[style.top]="'calc(1rem + var(--announcement-banner-height, 0px))'">
<div class="flex lg:hidden fixed top-4 left-4 z-50 gap-2">
<!-- Open sidenav button -->
<button
type="button"
Expand Down Expand Up @@ -111,12 +107,11 @@
[class.lg:pl-72]="!sidenavService.isCollapsed() && !chromeHidden()"
[class.lg:pl-0]="sidenavService.isCollapsed() || chromeHidden()"
[class.artifact-pane-open]="artifactPanelOpen()">
<main class="flex h-dvh flex-col">
<!-- Ambient announcement strip (spec §D1). A flex child rather than an
overlay, so the scrolling content below reflows instead of hiding
under it. It publishes its measured height as
--announcement-banner-height; the fixed chat topnav and the
floating sidenav controls offset against that. -->
<main class="relative flex h-dvh flex-col">
<!-- Ambient announcement strip (spec §D1). Positioned `absolute` against
this `relative` main, so showing or dismissing it never reflows the
page. `relative` is load-bearing: without it the pill would anchor
to the viewport and drift out from under the sidenav padding. -->
@if (showAnnouncementBanner()) {
<app-announcement-banner />
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,30 +28,11 @@ function makeAnnouncement(overrides: Partial<Announcement> = {}): Announcement {
describe('AnnouncementBannerComponent', () => {
let bannerItem: ReturnType<typeof signal<Announcement | null>>;
let ack: ReturnType<typeof vi.fn>;
let observed: Element[];

beforeEach(() => {
TestBed.resetTestingModule();
bannerItem = signal<Announcement | null>(null);
ack = vi.fn(async () => true);
observed = [];

// jsdom has no ResizeObserver. Stub one that records what it watches so
// the height-publishing path is actually exercised rather than skipped.
(globalThis as any).ResizeObserver = class {
constructor(private cb: ResizeObserverCallback) {}
observe(el: Element) {
observed.push(el);
}
disconnect() {}
unobserve() {}
emit(height: number) {
this.cb(
[{ contentRect: { height } } as unknown as ResizeObserverEntry],
this as unknown as ResizeObserver,
);
}
};

// DI-token override rather than vi.mock, per house convention.
TestBed.configureTestingModule({
Expand All @@ -63,7 +44,6 @@ describe('AnnouncementBannerComponent', () => {

afterEach(() => {
TestBed.resetTestingModule();
document.documentElement.style.removeProperty('--announcement-banner-height');
});

function create() {
Expand Down Expand Up @@ -204,29 +184,59 @@ describe('AnnouncementBannerComponent', () => {
expect(link.getAttribute('rel')).toBe('noopener noreferrer');
});

it('publishes its measured height so the fixed topnav can clear it', () => {
bannerItem.set(makeAnnouncement());
const fixture = create();
describe('overlays rather than occupying space', () => {
it('positions the host absolutely, so dismissing it cannot reflow the page', () => {
// The regression: the banner used to be a flex child of the shell's
// <main>, so showing or hiding it pulled the whole view up or down by
// its height. It must never participate in flow.
bannerItem.set(makeAnnouncement());
const fixture = create();
const host = fixture.nativeElement as HTMLElement;

expect(host.className).toContain('absolute');
expect(host.className).not.toContain('block');
});

expect(observed).toHaveLength(1);
expect(observed[0]).toBe(fixture.nativeElement);
});
it('lets clicks through the positioning strip to the chrome beneath', () => {
// The strip spans the full content width. Without this, it would
// swallow clicks aimed at the topnav and the sidebar buttons under it.
bannerItem.set(makeAnnouncement());
const fixture = create();
const host = fixture.nativeElement as HTMLElement;

it('clears the height variable on destroy, so nothing is left offset', () => {
bannerItem.set(makeAnnouncement());
const fixture = create();
expect(
document.documentElement.style.getPropertyValue(
'--announcement-banner-height',
),
).not.toBe('');
expect(host.className).toContain('pointer-events-none');
expect(strip(fixture)!.className).toContain('pointer-events-auto');
});

it('clears the topnav and the floating sidenav controls', () => {
// `top-16` puts the pill just below a chat route's fixed topnav, and
// below the shell's `top-4` sidebar buttons everywhere else — one
// constant instead of route awareness.
bannerItem.set(makeAnnouncement());
const fixture = create();
expect((fixture.nativeElement as HTMLElement).className).toContain('top-16');
});

fixture.destroy();
it('reads as a floating card, not a full-bleed strip', () => {
bannerItem.set(makeAnnouncement());
const fixture = create();
const pill = strip(fixture)!;

expect(
document.documentElement.style.getPropertyValue(
'--announcement-banner-height',
),
).toBe('');
expect(pill.className).toContain('rounded-2xl');
expect(pill.className).toContain('shadow-lg');
expect(pill.className).toContain('max-w-2xl');
// The old full-bleed look leaned on a bottom border instead.
expect(pill.className).not.toContain('border-b');
});

it('sets no layout variable on the document — nothing offsets against it', () => {
bannerItem.set(makeAnnouncement());
create();
expect(
document.documentElement.style.getPropertyValue(
'--announcement-banner-height',
),
).toBe('');
});
});
});
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
import {
ChangeDetectionStrategy,
Component,
DestroyRef,
ElementRef,
computed,
effect,
inject,
signal,
} from '@angular/core';
import { DOCUMENT } from '@angular/common';
import { NgIcon, provideIcons } from '@ng-icons/core';
import {
heroCheckCircle,
Expand Down Expand Up @@ -45,12 +41,23 @@ import {
* the POST fails, so a transient 500 cannot trap a user under an
* undismissable strip.
*
* **It publishes its own height** as `--announcement-banner-height` on the
* document root. The strip is a flex child of the shell's `<main>`, so the
* scrolling content reflows on its own — but the chat topnav is
* `position: fixed`, and would sit on top of the banner without that offset.
* The value is measured rather than hardcoded because the line wraps on narrow
* viewports.
* **It overlays rather than occupying space.** The host is positioned
* `absolute` inside the shell's `<main>`, so showing or dismissing it never
* reflows the page — an earlier version was a flex child, and dismissing it
* pulled the whole view up by its height. Overlaying also removes the reason
* three pieces of viewport-fixed chrome used to offset against a measured
* `--announcement-banner-height`: nothing has to move any more, so that
* variable, its `ResizeObserver`, and all three offsets are gone.
*
* `top-16` 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 sidenav controls, which sit at `top-4` and
* would otherwise be overlapped by a centred pill on any viewport narrow
* enough for the two to meet. One constant, no route awareness.
*
* The wrapper is `pointer-events-none` and only the pill itself takes events,
* so the full-width positioning strip cannot swallow clicks aimed at the
* topnav or the sidebar buttons underneath it.
*
* Body markdown is deliberately *not* rendered here: this surface is one line.
* The full body lives in What's New, which is why `panel` is forced onto every
Expand All @@ -68,11 +75,16 @@ import {
heroXMark,
}),
],
host: { class: 'block' },
host: {
// The positioning strip. `pointer-events-none` here (and `auto` on the
// pill) keeps it from swallowing clicks meant for the chrome beneath.
class:
'pointer-events-none absolute inset-x-0 top-16 z-50 flex justify-center px-4',
},
template: `
@if (announcement(); as item) {
<div
class="flex items-center gap-x-3 border-b px-4 py-2.5 sm:px-6"
class="pointer-events-auto flex w-full max-w-2xl items-center gap-x-3 rounded-2xl border px-4 py-2.5 shadow-lg sm:px-5"
[class]="severityClass()"
role="status"
aria-live="polite"
Expand Down Expand Up @@ -112,12 +124,6 @@ import {
})
export class AnnouncementBannerComponent {
private readonly announcements = inject(AnnouncementsService);
private readonly host: ElementRef<HTMLElement> = inject(ElementRef);
private readonly destroyRef = inject(DestroyRef);
private readonly document = inject(DOCUMENT);

/** The CSS custom property the shell's fixed chrome offsets against. */
private static readonly HEIGHT_VAR = '--announcement-banner-height';

readonly announcement = computed<Announcement | null>(() =>
this.announcements.bannerItem(),
Expand Down Expand Up @@ -175,42 +181,13 @@ export class AnnouncementBannerComponent {
() => this.announcement()?.severity ?? 'info',
);

/** Measured height of the strip, mirrored onto the document root. */
private readonly height = signal(0);

constructor() {
effect(() => {
const item = this.announcement();
if (!item || this.reportedSeen.has(item.announcement_id)) return;
this.reportedSeen.add(item.announcement_id);
void this.announcements.ack(item.announcement_id, 'seen', 'banner');
});

effect(() => {
const px = this.height();
this.document.documentElement.style.setProperty(
AnnouncementBannerComponent.HEIGHT_VAR,
`${px}px`,
);
});

// `ResizeObserver` rather than a one-shot measurement: the line wraps when
// the viewport narrows or the artifact pane opens, and the fixed topnav
// has to follow it down.
if (typeof ResizeObserver !== 'undefined') {
const observer = new ResizeObserver(entries => {
const next = entries[0]?.contentRect.height ?? 0;
this.height.set(Math.round(next));
});
observer.observe(this.host.nativeElement);
this.destroyRef.onDestroy(() => observer.disconnect());
}

this.destroyRef.onDestroy(() => {
this.document.documentElement.style.removeProperty(
AnnouncementBannerComponent.HEIGHT_VAR,
);
});
}

protected onDismiss(): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,7 @@
/* Topnav wrapper for full-page mode */
.chat-topnav-wrapper {
position: fixed;
/* Sits directly below the announcement strip when one is showing. The strip
is a flex child of the shell's <main>, so the scroll container already
starts below it — but this bar is fixed and would otherwise cover it.
The variable is published (and cleared) by app-announcement-banner. */
top: var(--announcement-banner-height, 0px);
top: 0;
left: 0;
right: 0;
z-index: 40;
Expand Down Expand Up @@ -53,13 +49,7 @@
/* Empty state container for full-page mode */
.chat-container-empty.full-page {
position: fixed;
/* Not `inset: 0` — this overlay is viewport-fixed and opaque, so a flush
top would paint over the announcement strip that sits above the shell's
scroll container. Same offset the topnav takes. */
top: var(--announcement-banner-height, 0px);
right: 0;
bottom: 0;
left: 0;
inset: 0;
display: flex;
align-items: center;
justify-content: center;
Expand Down