From d460e8ac33d62ac2a12bb1fe9a9a676caea1bf66 Mon Sep 17 00:00:00 2001
From: Josh Knapp <jknapp85@gmail.com>
Date: Wed, 15 Jul 2026 21:52:11 -0700
Subject: [PATCH 1/2] topbar: surface publish warnings instead of discarding
 them

handlePublish's JSON response has always included a `warnings` array
(e.g. the contact-form relay's "submissions will not be delivered"
notice), but nothing in the editor ever read it. Adds a PublishWarnings
banner, held in its own state independent of the 3s publishStatus
flash so the customer has time to read it, rendered in both the
desktop and mobile TopBar branches.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 .../panels/topbar/PublishWarnings.test.tsx    | 57 +++++++++++++++++++
 craft/src/panels/topbar/PublishWarnings.tsx   | 31 ++++++++++
 craft/src/panels/topbar/TopBar.tsx            |  9 +++
 craft/src/styles/editor.css                   | 35 ++++++++++++
 4 files changed, 132 insertions(+)
 create mode 100644 craft/src/panels/topbar/PublishWarnings.test.tsx
 create mode 100644 craft/src/panels/topbar/PublishWarnings.tsx

diff --git a/craft/src/panels/topbar/PublishWarnings.test.tsx b/craft/src/panels/topbar/PublishWarnings.test.tsx
new file mode 100644
index 0000000..7fd309d
--- /dev/null
+++ b/craft/src/panels/topbar/PublishWarnings.test.tsx
@@ -0,0 +1,57 @@
+import { describe, test, expect, vi, afterEach } from 'vitest';
+import React from 'react';
+import { createRoot, Root } from 'react-dom/client';
+import { act } from 'react-dom/test-utils';
+import { PublishWarnings } from './PublishWarnings';
+
+/* ---------- DOM test harness (no @testing-library/react in this repo, see
+   src/ui/AssetPicker.test.tsx for the same pattern: react-dom/client +
+   react-dom/test-utils `act`, both transitive deps of react-dom already). ---------- */
+let container: HTMLDivElement;
+let root: Root;
+
+function render(ui: React.ReactElement) {
+  container = document.createElement('div');
+  document.body.appendChild(container);
+  act(() => {
+    root = createRoot(container);
+    root.render(ui);
+  });
+}
+
+function click(el: Element | null) {
+  if (!el) throw new Error('element not found');
+  act(() => { (el as HTMLElement).dispatchEvent(new MouseEvent('click', { bubbles: true })); });
+}
+
+afterEach(() => {
+  if (container) {
+    act(() => { root.unmount(); });
+    container.remove();
+  }
+});
+
+describe('PublishWarnings', () => {
+  test('renders nothing when there are no warnings', () => {
+    render(<PublishWarnings warnings={[]} onDismiss={() => {}} />);
+    expect(container.innerHTML).toBe('');
+  });
+
+  test('renders each warning', () => {
+    render(
+      <PublishWarnings
+        warnings={['First problem.', 'Second problem.']}
+        onDismiss={() => {}}
+      />,
+    );
+    expect(container.textContent).toContain('First problem.');
+    expect(container.textContent).toContain('Second problem.');
+  });
+
+  test('dismiss fires the callback', () => {
+    const onDismiss = vi.fn();
+    render(<PublishWarnings warnings={['A problem.']} onDismiss={onDismiss} />);
+    click(container.querySelector('[data-testid="publish-warnings-dismiss"]'));
+    expect(onDismiss).toHaveBeenCalledTimes(1);
+  });
+});
diff --git a/craft/src/panels/topbar/PublishWarnings.tsx b/craft/src/panels/topbar/PublishWarnings.tsx
new file mode 100644
index 0000000..3a0a81e
--- /dev/null
+++ b/craft/src/panels/topbar/PublishWarnings.tsx
@@ -0,0 +1,31 @@
+import React from 'react';
+
+export interface PublishWarningsProps {
+  warnings: string[];
+  onDismiss: () => void;
+}
+
+/** Non-blocking banner shown after a successful publish. The site IS live --
+ *  these are things the customer should fix and re-publish, not failures. */
+export const PublishWarnings: React.FC<PublishWarningsProps> = ({ warnings, onDismiss }) => {
+  if (!warnings.length) return null;
+
+  return (
+    <div className="publish-warnings" role="status" data-testid="publish-warnings">
+      <i className="fa fa-exclamation-triangle" aria-hidden="true" />
+      <ul>
+        {warnings.map((w, i) => (
+          <li key={i}>{w}</li>
+        ))}
+      </ul>
+      <button
+        type="button"
+        onClick={onDismiss}
+        aria-label="Dismiss"
+        data-testid="publish-warnings-dismiss"
+      >
+        <i className="fa fa-times" aria-hidden="true" />
+      </button>
+    </div>
+  );
+};
diff --git a/craft/src/panels/topbar/TopBar.tsx b/craft/src/panels/topbar/TopBar.tsx
index ceac12f..bba65db 100644
--- a/craft/src/panels/topbar/TopBar.tsx
+++ b/craft/src/panels/topbar/TopBar.tsx
@@ -10,6 +10,7 @@ import { DeviceMode } from '../../types';
 import { TemplateModal } from './TemplateModal';
 import { HeadCodeModal } from './HeadCodeModal';
 import { TopBarOverflowMenu } from './TopBarOverflowMenu';
+import { PublishWarnings } from './PublishWarnings';
 import { SitesmithButton } from '../sitesmith/SitesmithButton';
 import { useSitesmithModal } from '../../state/SitesmithContext';
 
@@ -33,6 +34,7 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
 
   const [saveStatus, setSaveStatus] = useState<'idle' | 'saving' | 'saved' | 'error'>('idle');
   const [publishStatus, setPublishStatus] = useState<'idle' | 'publishing' | 'published' | 'error'>('idle');
+  const [publishWarnings, setPublishWarnings] = useState<string[]>([]);
   const [isDraft, setIsDraft] = useState(false);
   // Mobile-A2: lifted from private useState into MobileChromeContext so
   // opening a mobile sheet can close these modals (item 3) -- behavior is
@@ -100,11 +102,16 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
 
   const handlePublish = useCallback(async () => {
     setPublishStatus('publishing');
+    setPublishWarnings([]);
     try {
       const result = await publish();
       if (result?.success) {
         setPublishStatus('published');
         setIsDraft(false);
+        // The site published; these are fixable problems, not failures. Held
+        // independently of publishStatus so the 3s "Published" flash doesn't
+        // take the warning down with it.
+        setPublishWarnings(Array.isArray(result.warnings) ? result.warnings : []);
         if (publishTimeoutRef.current) clearTimeout(publishTimeoutRef.current);
         publishTimeoutRef.current = setTimeout(() => setPublishStatus('idle'), 3000);
       } else {
@@ -220,6 +227,7 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
   if (isMobile) {
     return (
       <nav className="topbar topbar-mobile">
+        <PublishWarnings warnings={publishWarnings} onDismiss={() => setPublishWarnings([])} />
         <div className="topbar-left">
           {isWHP && (
             <a
@@ -307,6 +315,7 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
 
   return (
     <nav className="topbar">
+      <PublishWarnings warnings={publishWarnings} onDismiss={() => setPublishWarnings([])} />
       <div className="topbar-left">
         {isWHP && (
           <a href={whpConfig!.backUrl} className="topbar-btn back-btn" aria-label="Back to Panel">
diff --git a/craft/src/styles/editor.css b/craft/src/styles/editor.css
index d2cfad1..1d5072d 100644
--- a/craft/src/styles/editor.css
+++ b/craft/src/styles/editor.css
@@ -106,6 +106,11 @@ body {
   border-bottom: 1px solid var(--color-border);
   z-index: 100;
   gap: 12px;
+  /* Positioned ancestor for .publish-warnings (position: absolute; top:
+     100%), which is rendered as this <nav>'s first child in both the
+     desktop and mobile branches -- without this, it would anchor to the
+     viewport instead of sitting directly under the topbar. */
+  position: relative;
 }
 
 .topbar-left,
@@ -1884,3 +1889,33 @@ body {
     height: 44px !important;
   }
 }
+
+/* --------------------------------------------------------------------------
+   Publish warnings banner -- non-blocking; the site DID publish. Anchored
+   to .topbar's `position: relative` (see above) so it drops down directly
+   beneath the bar in both the desktop and mobile branches.
+   -------------------------------------------------------------------------- */
+.publish-warnings {
+  position: absolute;
+  top: 100%;
+  left: 0;
+  right: 0;
+  z-index: 40;
+  display: flex;
+  align-items: flex-start;
+  gap: 8px;
+  padding: 10px 12px;
+  background: #422006;
+  border-bottom: 1px solid #a16207;
+  color: #fde68a;
+  font-size: 12px;
+}
+.publish-warnings ul { margin: 0; padding-left: 16px; flex: 1; }
+.publish-warnings li { margin: 2px 0; }
+.publish-warnings button {
+  background: none;
+  border: none;
+  color: #fde68a;
+  cursor: pointer;
+  padding: 0 4px;
+}

From c712a69c4ad2a51cea6afd9748f3356815124875 Mon Sep 17 00:00:00 2001
From: Josh Knapp <jknapp85@gmail.com>
Date: Wed, 15 Jul 2026 22:12:31 -0700
Subject: [PATCH 2/2] test(topbar): cover publish() warnings wiring into
 PublishWarnings banner

PublishWarnings.tsx had unit tests for the presentational banner, but
nothing asserted that result.warnings from publish() actually flows
through TopBar's handlePublish into it. That 3-line seam is exactly what
this feature exists to fix -- the backend always returned warnings, and
TopBar discarded them by only checking result.success, so the
contact-form-relay warning was dead code for its entire life.

Adds TopBar.test.tsx asserting: warnings render after a successful
publish with warnings, no banner renders when warnings is absent, a
warning doesn't present as a publish failure, warnings survive the 3s
"Published" flash (fake timers, advanced past 3000ms), and a fresh
publish clears stale warnings from the previous one.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 craft/src/panels/topbar/TopBar.test.tsx | 191 ++++++++++++++++++++++++
 1 file changed, 191 insertions(+)
 create mode 100644 craft/src/panels/topbar/TopBar.test.tsx

diff --git a/craft/src/panels/topbar/TopBar.test.tsx b/craft/src/panels/topbar/TopBar.test.tsx
new file mode 100644
index 0000000..9a94b19
--- /dev/null
+++ b/craft/src/panels/topbar/TopBar.test.tsx
@@ -0,0 +1,191 @@
+import { describe, test, expect, vi, afterEach } from 'vitest';
+import React from 'react';
+import { createRoot, Root } from 'react-dom/client';
+import { act } from 'react-dom/test-utils';
+
+/**
+ * Closes the gap flagged by the whole-branch review: PublishWarnings.test.tsx
+ * covers the presentational banner in isolation, but nothing asserted that
+ * `result.warnings` from `publish()` actually reaches it through TopBar. That
+ * 3-line seam (handlePublish -> setPublishWarnings -> <PublishWarnings>) is
+ * exactly what regressed before this feature existed: the backend has always
+ * returned `warnings`, and TopBar discarded them by only checking
+ * `result.success` -- so the contact-form-relay warning was dead code for its
+ * entire life. This suite drives the real `<TopBar>` through a real button
+ * click and asserts the warnings show up, survive the 3s "Published" flash,
+ * don't taint the success/error status, and get cleared by a fresh publish.
+ *
+ * Mocks (same DOM-harness + `vi.mock('@craftjs/core', ...)` pattern as
+ * RenderNode.test.tsx / useWhpApi.load.test.tsx -- no @testing-library/react
+ * in this repo):
+ *  - `@craftjs/core`'s `useEditor`: TopBar only needs inert undo/redo/query
+ *    stubs, not a real Craft.js tree.
+ *  - `useWhpApi`: this IS the seam under test -- `publish` is a controllable
+ *    mock so each test can choose exactly what the "backend" returns.
+ *  - TemplateModal / HeadCodeModal / SitesmithButton: sibling chrome
+ *    unrelated to the warnings seam (portals, CodeMirror lazy-load,
+ *    useSitesmith's own fetch calls) -- stubbed out so this suite stays
+ *    focused, same as MobilePanelBar.test.tsx stubbing its sibling panels.
+ * `usePages`/`useSiteDesign`/`useMobileChrome` are left un-mocked and
+ * un-provided -- their default context values (defined in each context
+ * module) are harmless no-op stubs, and TopBar's desktop render path never
+ * needs more than that.
+ */
+
+vi.mock('@craftjs/core', () => ({
+  useEditor: (collector?: (state: unknown, query: unknown) => Record<string, unknown>) => {
+    const query = {
+      serialize: () => '{}',
+      history: { canUndo: () => false, canRedo: () => false },
+    };
+    const actions = { history: { undo: vi.fn(), redo: vi.fn() } };
+    const collected = collector ? collector({}, query) : {};
+    return { actions, query, ...collected };
+  },
+}));
+
+const publishMock = vi.fn();
+vi.mock('../../hooks/useWhpApi', () => ({
+  useWhpApi: () => ({
+    save: vi.fn().mockResolvedValue({ success: true }),
+    publish: publishMock,
+    load: vi.fn().mockResolvedValue(null),
+    uploadAsset: vi.fn(),
+    isWHP: true,
+  }),
+}));
+
+vi.mock('./TemplateModal', () => ({ TemplateModal: () => null }));
+vi.mock('./HeadCodeModal', () => ({ HeadCodeModal: () => null }));
+vi.mock('../sitesmith/SitesmithButton', () => ({ SitesmithButton: () => null }));
+
+import { TopBar } from './TopBar';
+import { EditorConfigProvider } from '../../state/EditorConfigContext';
+import { WhpConfig } from '../../types';
+
+const whpConfig: WhpConfig = {
+  user: 'testuser',
+  apiUrl: '/panel/api/site-builder',
+  csrfToken: 'tok',
+  siteId: 1,
+  siteDomain: 'example.com',
+  siteName: 'Example Site',
+  backUrl: '/panel/sites',
+  isRoot: false,
+};
+
+let container: HTMLDivElement;
+let root: Root;
+
+function render() {
+  container = document.createElement('div');
+  document.body.appendChild(container);
+  act(() => {
+    root = createRoot(container);
+    root.render(
+      <EditorConfigProvider config={whpConfig}>
+        <TopBar device="desktop" onDeviceChange={() => {}} showGuides={false} onToggleGuides={() => {}} />
+      </EditorConfigProvider>,
+    );
+  });
+}
+
+function publishButton(): HTMLButtonElement {
+  const btn = container.querySelector<HTMLButtonElement>('.topbar-btn.publish');
+  if (!btn) throw new Error('Publish button not found');
+  return btn;
+}
+
+/** handlePublish does exactly one `await publish()` before touching state;
+ *  two microtask flushes inside the same act() batch is enough to carry that
+ *  through to the resulting re-render. */
+async function clickPublish() {
+  await act(async () => {
+    publishButton().dispatchEvent(new MouseEvent('click', { bubbles: true }));
+    await Promise.resolve();
+    await Promise.resolve();
+  });
+}
+
+afterEach(() => {
+  if (container) {
+    act(() => { root.unmount(); });
+    container.remove();
+  }
+  publishMock.mockReset();
+  vi.useRealTimers();
+});
+
+describe('TopBar publish-warnings wiring', () => {
+  test('warnings from publish() flow through into the rendered banner', async () => {
+    publishMock.mockResolvedValue({ success: true, warnings: ['Warning one.', 'Warning two.'] });
+    render();
+
+    await clickPublish();
+
+    expect(container.textContent).toContain('Warning one.');
+    expect(container.textContent).toContain('Warning two.');
+  });
+
+  test('a clean publish (no warnings key) renders no banner at all', async () => {
+    publishMock.mockResolvedValue({ success: true });
+    render();
+
+    await clickPublish();
+
+    expect(container.querySelector('[data-testid="publish-warnings"]')).toBeNull();
+  });
+
+  test('a warning is non-blocking -- publish still reports success, not a failure', async () => {
+    publishMock.mockResolvedValue({
+      success: true,
+      warnings: ['Submissions will not be delivered until an administrator enables it.'],
+    });
+    render();
+
+    await clickPublish();
+
+    expect(container.querySelector('.publish-badge.published')).not.toBeNull();
+    expect(container.querySelector('.save-indicator.error')).toBeNull();
+    expect(container.textContent).toContain('Submissions will not be delivered until an administrator enables it.');
+  });
+
+  test('warnings survive the 3-second "Published" flash timer', async () => {
+    vi.useFakeTimers();
+    publishMock.mockResolvedValue({ success: true, warnings: ['Sticks around after the flash.'] });
+    render();
+
+    await act(async () => {
+      publishButton().dispatchEvent(new MouseEvent('click', { bubbles: true }));
+      await Promise.resolve();
+      await Promise.resolve();
+    });
+
+    // Sanity: both the flash and the warning are up before the timer fires.
+    expect(container.querySelector('.publish-badge.published')).not.toBeNull();
+    expect(container.textContent).toContain('Sticks around after the flash.');
+
+    act(() => {
+      vi.advanceTimersByTime(3000);
+    });
+
+    // The 3s timer resets publishStatus -> the "Published" flash is gone...
+    expect(container.querySelector('.publish-badge.published')).toBeNull();
+    // ...but publishWarnings lives in its own state and must NOT have been
+    // cleared by that same timer.
+    expect(container.textContent).toContain('Sticks around after the flash.');
+  });
+
+  test('a new publish attempt clears warnings left over from the previous one', async () => {
+    publishMock.mockResolvedValueOnce({ success: true, warnings: ['Old warning.'] });
+    render();
+    await clickPublish();
+    expect(container.textContent).toContain('Old warning.');
+
+    publishMock.mockResolvedValueOnce({ success: true });
+    await clickPublish();
+
+    expect(container.textContent).not.toContain('Old warning.');
+    expect(container.querySelector('[data-testid="publish-warnings"]')).toBeNull();
+  });
+});