feat(announcements): open the full announcement from the banner text

docs/specs/feature-announcements.md

@@ -88,6 +88,25 @@ An announcement carries `surfaces: list[Literal["panel", "banner", "modal"]]`.
   rendered by a sibling of `quota-warning-banner`. One line plus an optional
   CTA and a ✕.
 
+  **Its text is a button that opens the announcement's own dialog.** The pill
+  renders no body, so without a way in, its only affordances are ✕ and an
+  optional external CTA — which trains people to dismiss unread, with What's
+  New (buried in the user menu) as the only other route to the content. It
+  opens the single-announcement dialog rather than the What's New list
+  deliberately: the pill named one thing, and handing back a list to search
+  through is a worse answer than the thing itself. The dialog owns the ack
+  from that point (`sourceSurface: "banner"`, so reach stats can tell a
+  banner-driven read from an interruption), and any of its exits retires the
+  pill durably — having read the body is a stronger signal of consumption
+  than clicking ✕ on a one-line strip.
+
+  **A `requiresAck` announcement gets no ✕ on the pill.** Dismissal
+  suppression is rank-based and covers the banner *and* modal slots alike
+  (§D7), so a ✕ here would let a user retire a compliance notice from the
+  strip before the blocking modal ever fired — leaving no `acknowledged`
+  record anywhere. On those, the only way out is to open it and press the
+  button.
+
   *Revised after PR-4 shipped.* It was first built as a full-bleed strip below
   the top nav. Two things moved it. Dismissing a strip that occupied layout
   reflowed the whole view, so it became an overlay; and what a banner
@@ -128,6 +147,11 @@ bust. **Announcements never touch the model call path** (D12).
 | `dismissed` | User clicks ✕ or "Got it" | Suppresses banner and modal. Entry stays in the panel. |
 | `acknowledged` | User clicks the confirm button on a `requiresAck` modal | As `dismissed`, plus it is a durable record an admin can report on. |
 
+The ack's `surface` records **where the gesture happened**, not which surface
+owns the announcement: a dialog the user opened by clicking the banner's text
+writes `banner` for every action it records, so reach stats can distinguish a
+banner that earned a read from a modal the user never asked for.
+
 They are ranked (`seen=1 < dismissed=2 < acknowledged=3`) and the stored rank
 **only ever increases**. The write is a conditional `UpdateExpression`:
 

frontend/ai.client/src/app/components/announcement-banner/announcement-banner.component.spec.ts

@@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
 import { TestBed } from '@angular/core/testing';
 import { signal } from '@angular/core';
 import { AnnouncementsService } from '../../services/announcements/announcements.service';
+import { AnnouncementModalService } from '../../services/announcements/announcement-modal.service';
 import { Announcement } from '../../services/announcements/announcement.model';
 import { AnnouncementBannerComponent } from './announcement-banner.component';
 
@@ -28,16 +29,19 @@ function makeAnnouncement(overrides: Partial<Announcement> = {}): Announcement {
 describe('AnnouncementBannerComponent', () => {
   let bannerItem: ReturnType<typeof signal<Announcement | null>>;
   let ack: ReturnType<typeof vi.fn>;
+  let openFor: ReturnType<typeof vi.fn>;
 
   beforeEach(() => {
     TestBed.resetTestingModule();
     bannerItem = signal<Announcement | null>(null);
     ack = vi.fn(async () => true);
+    openFor = vi.fn();
 
     // DI-token override rather than vi.mock, per house convention.
     TestBed.configureTestingModule({
       providers: [
         { provide: AnnouncementsService, useValue: { bannerItem, ack } },
+        { provide: AnnouncementModalService, useValue: { openFor } },
       ],
     });
   });
@@ -127,12 +131,11 @@ describe('AnnouncementBannerComponent', () => {
     const fixture = create();
     ack.mockClear();
 
+    // By label, not "the first button" — the text is a button too now.
     const dismiss = (fixture.nativeElement as HTMLElement).querySelector(
-      'button',
+      'button[aria-label="Dismiss announcement: Skills are here"]',
     ) as HTMLButtonElement;
-    expect(dismiss.getAttribute('aria-label')).toBe(
-      'Dismiss announcement: Skills are here',
-    );
+    expect(dismiss).not.toBeNull();
     dismiss.click();
 
     expect(ack).toHaveBeenCalledWith('a1', 'dismissed', 'banner');
@@ -173,6 +176,7 @@ describe('AnnouncementBannerComponent', () => {
     TestBed.configureTestingModule({
       providers: [
         { provide: AnnouncementsService, useValue: { bannerItem, ack } },
+        { provide: AnnouncementModalService, useValue: { openFor } },
       ],
     });
     bannerItem.set(
@@ -184,6 +188,103 @@ describe('AnnouncementBannerComponent', () => {
     expect(link.getAttribute('rel')).toBe('noopener noreferrer');
   });
 
+  describe('the text opens the full announcement', () => {
+    function readMore(fixture: ReturnType<typeof create>) {
+      // By `title`, which carries the untruncated headline — the ✕ is the one
+      // with an aria-label.
+      return (fixture.nativeElement as HTMLElement).querySelector(
+        'button[title="Skills are here"]',
+      ) as HTMLButtonElement | null;
+    }
+
+    it('hands the announcement to the modal service, attributed to the banner', () => {
+      // The pill renders no body, so without this the only affordances on it
+      // are ✕ and an optional CTA — which trains people to dismiss unread.
+      bannerItem.set(makeAnnouncement());
+      const fixture = create();
+
+      readMore(fixture)!.click();
+
+      expect(openFor).toHaveBeenCalledWith(
+        expect.objectContaining({ announcement_id: 'a1' }),
+        'banner',
+      );
+    });
+
+    it('writes no ack of its own — the dialog owns that', () => {
+      // Acking `dismissed` here as well would be a second, racing write for
+      // the same gesture, and would retire the pill before the user has read
+      // a word of the body.
+      bannerItem.set(makeAnnouncement());
+      const fixture = create();
+      ack.mockClear();
+
+      readMore(fixture)!.click();
+
+      expect(ack).not.toHaveBeenCalled();
+    });
+
+    it('keeps the visible text inside the accessible name (WCAG 2.5.3)', () => {
+      // `bannerText()` may be the summary, so an aria-label built from the
+      // title would leave a voice-control user saying a phrase that is not
+      // the button's name. The visible words have to survive.
+      bannerItem.set(makeAnnouncement({ summary: 'Short version' }));
+      const fixture = create();
+      const button = readMore(fixture)!;
+
+      expect(button.getAttribute('aria-label')).toBeNull();
+      expect(button.textContent).toContain('Short version');
+      expect(button.textContent).toContain('Read more');
+      // The untruncated headline is still reachable on hover.
+      expect(button.getAttribute('title')).toBe('Skills are here');
+    });
+  });
+
+  describe('a requiresAck announcement cannot be retired from the strip', () => {
+    it('offers no ✕', () => {
+      // `dismissed` and `acknowledged` both sit at or above SUPPRESSING_RANK,
+      // and suppression covers the banner AND modal slots — so a ✕ here would
+      // let a user kill a compliance notice before the blocking modal ever
+      // fired, leaving no `acknowledged` record anywhere.
+      bannerItem.set(makeAnnouncement({ requires_ack: true }));
+      const fixture = create();
+
+      expect(
+        (fixture.nativeElement as HTMLElement).querySelector(
+          'button[aria-label^="Dismiss announcement"]',
+        ),
+      ).toBeNull();
+    });
+
+    it('still opens on click — that is the only way out', () => {
+      bannerItem.set(makeAnnouncement({ requires_ack: true }));
+      const fixture = create();
+
+      (
+        (fixture.nativeElement as HTMLElement).querySelector(
+          'button[title="Skills are here"]',
+        ) as HTMLButtonElement
+      ).click();
+
+      expect(openFor).toHaveBeenCalledWith(
+        expect.objectContaining({ requires_ack: true }),
+        'banner',
+      );
+    });
+
+    it('writes no `dismissed` even if onDismiss is reached some other way', () => {
+      bannerItem.set(makeAnnouncement({ requires_ack: true }));
+      const fixture = create();
+      ack.mockClear();
+
+      (
+        fixture.componentInstance as unknown as { onDismiss(): void }
+      ).onDismiss();
+
+      expect(ack).not.toHaveBeenCalled();
+    });
+  });
+
   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

frontend/ai.client/src/app/components/announcement-banner/announcement-banner.component.ts

@@ -14,6 +14,7 @@ import {
   heroXMark,
 } from '@ng-icons/heroicons/outline';
 import { AnnouncementsService } from '../../services/announcements/announcements.service';
+import { AnnouncementModalService } from '../../services/announcements/announcement-modal.service';
 import {
   Announcement,
   AnnouncementSeverity,
@@ -80,6 +81,22 @@ import {
  * 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
  * announcement server-side.
+ *
+ * **The text is a button, and clicking it opens the full announcement.**
+ * Without it the only affordances on the pill are ✕ and an optional CTA, which
+ * trains people to dismiss unread — and What's New, the only other way to the
+ * body, is buried in the user menu. It opens the single-announcement dialog
+ * rather than the What's New list on purpose: the pill named one thing, so
+ * handing back a list to search is a worse answer than the thing itself. From
+ * there the dialog owns the ack, and any of its exits retires the pill
+ * durably (see `onOpenDetail`).
+ *
+ * **A `requiresAck` announcement gets no ✕ here.** `dismissed` and
+ * `acknowledged` are both at or above `SUPPRESSING_RANK`, and suppression
+ * applies to the banner *and* modal slots alike — so with a ✕ on the strip, a
+ * user could retire a compliance notice from the pill and the blocking modal
+ * would never fire, leaving no `acknowledged` record anywhere. On those, the
+ * only way out is to open it and press the button.
  */
 @Component({
   selector: 'app-announcement-banner',
@@ -117,9 +134,19 @@ import {
           aria-hidden="true"
         />
 
-        <p class="min-w-0 truncate font-medium">
-          {{ bannerText() }}
-        </p>
+        <button
+          type="button"
+          (click)="onOpenDetail()"
+          [attr.title]="item.title"
+          class="min-w-0 cursor-pointer truncate text-left font-medium underline-offset-2 hover:underline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-current"
+        >
+          <!-- Not an aria-label: that would replace the visible text with a
+               string the summary may not contain, and WCAG 2.5.3 (Label in
+               Name) wants the visible label inside the accessible name — a
+               voice-control user says what they can see. Prefixing keeps
+               both. -->
+          <span class="sr-only">Read more: </span>{{ bannerText() }}
+        </button>
 
         @if (item.cta_url && item.cta_label) {
           <a
@@ -132,20 +159,23 @@ import {
           </a>
         }
 
-        <button
-          type="button"
-          (click)="onDismiss()"
-          [attr.aria-label]="'Dismiss announcement: ' + item.title"
-          class="-mr-1 flex size-5 shrink-0 items-center justify-center rounded-full hover:bg-black/10 focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-current dark:hover:bg-white/10"
-        >
-          <ng-icon name="heroXMark" class="size-3.5" aria-hidden="true" />
-        </button>
+        @if (!item.requires_ack) {
+          <button
+            type="button"
+            (click)="onDismiss()"
+            [attr.aria-label]="'Dismiss announcement: ' + item.title"
+            class="-mr-1 flex size-5 shrink-0 items-center justify-center rounded-full hover:bg-black/10 focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-current dark:hover:bg-white/10"
+          >
+            <ng-icon name="heroXMark" class="size-3.5" aria-hidden="true" />
+          </button>
+        }
       </div>
     }
   `,
 })
 export class AnnouncementBannerComponent {
   private readonly announcements = inject(AnnouncementsService);
+  private readonly modals = inject(AnnouncementModalService);
 
   /**
    * Which side of the composer to take. `'above'` suits a bottom-pinned
@@ -221,7 +251,24 @@ export class AnnouncementBannerComponent {
 
   protected onDismiss(): void {
     const item = this.announcement();
-    if (!item) return;
+    if (!item || item.requires_ack) return;
     void this.announcements.ack(item.announcement_id, 'dismissed', 'banner');
   }
+
+  /**
+   * Open the full announcement, attributing whatever the user does there to
+   * the banner.
+   *
+   * The dialog owns the ack from here: any of its exits writes `dismissed`
+   * (or `acknowledged`), which outranks the `seen` this strip already wrote
+   * and retires the pill on every device. That is the intent — having read
+   * the body is a stronger signal of consumption than clicking ✕ on a
+   * one-line strip, and leaving the pill up afterwards would just ask the
+   * user to dismiss something they have already dealt with.
+   */
+  protected onOpenDetail(): void {
+    const item = this.announcement();
+    if (!item) return;
+    this.modals.openFor(item, 'banner');
+  }
 }

frontend/ai.client/src/app/components/announcement-modal/announcement-modal.component.spec.ts

@@ -3,7 +3,10 @@ import { TestBed } from '@angular/core/testing';
 import { DIALOG_DATA, DialogRef } from '@angular/cdk/dialog';
 import { provideMarkdown } from 'ngx-markdown';
 import { AnnouncementsService } from '../../services/announcements/announcements.service';
-import { Announcement } from '../../services/announcements/announcement.model';
+import {
+  Announcement,
+  AnnouncementSurface,
+} from '../../services/announcements/announcement.model';
 import {
   AnnouncementModalComponent,
   AnnouncementModalData,
@@ -33,7 +36,10 @@ describe('AnnouncementModalComponent', () => {
   let ack: ReturnType<typeof vi.fn>;
   let close: ReturnType<typeof vi.fn>;
 
-  function setup(announcement: Announcement) {
+  function setup(
+    announcement: Announcement,
+    sourceSurface?: AnnouncementSurface,
+  ) {
     TestBed.resetTestingModule();
     ack = vi.fn(async () => true);
     close = vi.fn();
@@ -45,7 +51,10 @@ describe('AnnouncementModalComponent', () => {
         { provide: DialogRef, useValue: { close, closed: { subscribe: vi.fn() } } },
         {
           provide: DIALOG_DATA,
-          useValue: { announcement } satisfies AnnouncementModalData,
+          useValue: {
+            announcement,
+            sourceSurface,
+          } satisfies AnnouncementModalData,
         },
       ],
     });
@@ -172,6 +181,33 @@ describe('AnnouncementModalComponent', () => {
     expect(link.getAttribute('rel')).toBe('noopener noreferrer');
   });
 
+  describe('ack attribution', () => {
+    it('defaults to the `modal` surface — the §D8 interruption', () => {
+      setup(makeAnnouncement());
+      expect(ack).toHaveBeenCalledWith('a1', 'seen', 'modal');
+    });
+
+    it('attributes every ack to the surface the user came from', () => {
+      // Opened by clicking the banner text. The ack row's `surface` is the
+      // only record of what drove the dismissal, so it must say `banner` —
+      // otherwise banner engagement is indistinguishable from an interruption
+      // the user never asked for.
+      const fixture = setup(makeAnnouncement(), 'banner');
+      expect(ack).toHaveBeenCalledWith('a1', 'seen', 'banner');
+
+      ack.mockClear();
+      confirmButton(fixture).click();
+      expect(ack).toHaveBeenCalledWith('a1', 'dismissed', 'banner');
+    });
+
+    it('carries the surface through an `acknowledged` too', () => {
+      const fixture = setup(makeAnnouncement({ requires_ack: true }), 'banner');
+      ack.mockClear();
+      confirmButton(fixture).click();
+      expect(ack).toHaveBeenCalledWith('a1', 'acknowledged', 'banner');
+    });
+  });
+
   it('is a labelled modal dialog', () => {
     const fixture = setup(makeAnnouncement());
     const panel = el(fixture).querySelector('[role="dialog"]')!;