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>
This commit is contained in:
@@ -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);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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>
|
||||||
|
);
|
||||||
|
};
|
||||||
@@ -10,6 +10,7 @@ import { DeviceMode } from '../../types';
|
|||||||
import { TemplateModal } from './TemplateModal';
|
import { TemplateModal } from './TemplateModal';
|
||||||
import { HeadCodeModal } from './HeadCodeModal';
|
import { HeadCodeModal } from './HeadCodeModal';
|
||||||
import { TopBarOverflowMenu } from './TopBarOverflowMenu';
|
import { TopBarOverflowMenu } from './TopBarOverflowMenu';
|
||||||
|
import { PublishWarnings } from './PublishWarnings';
|
||||||
import { SitesmithButton } from '../sitesmith/SitesmithButton';
|
import { SitesmithButton } from '../sitesmith/SitesmithButton';
|
||||||
import { useSitesmithModal } from '../../state/SitesmithContext';
|
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 [saveStatus, setSaveStatus] = useState<'idle' | 'saving' | 'saved' | 'error'>('idle');
|
||||||
const [publishStatus, setPublishStatus] = useState<'idle' | 'publishing' | 'published' | 'error'>('idle');
|
const [publishStatus, setPublishStatus] = useState<'idle' | 'publishing' | 'published' | 'error'>('idle');
|
||||||
|
const [publishWarnings, setPublishWarnings] = useState<string[]>([]);
|
||||||
const [isDraft, setIsDraft] = useState(false);
|
const [isDraft, setIsDraft] = useState(false);
|
||||||
// Mobile-A2: lifted from private useState into MobileChromeContext so
|
// Mobile-A2: lifted from private useState into MobileChromeContext so
|
||||||
// opening a mobile sheet can close these modals (item 3) -- behavior is
|
// 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 () => {
|
const handlePublish = useCallback(async () => {
|
||||||
setPublishStatus('publishing');
|
setPublishStatus('publishing');
|
||||||
|
setPublishWarnings([]);
|
||||||
try {
|
try {
|
||||||
const result = await publish();
|
const result = await publish();
|
||||||
if (result?.success) {
|
if (result?.success) {
|
||||||
setPublishStatus('published');
|
setPublishStatus('published');
|
||||||
setIsDraft(false);
|
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);
|
if (publishTimeoutRef.current) clearTimeout(publishTimeoutRef.current);
|
||||||
publishTimeoutRef.current = setTimeout(() => setPublishStatus('idle'), 3000);
|
publishTimeoutRef.current = setTimeout(() => setPublishStatus('idle'), 3000);
|
||||||
} else {
|
} else {
|
||||||
@@ -220,6 +227,7 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
|
|||||||
if (isMobile) {
|
if (isMobile) {
|
||||||
return (
|
return (
|
||||||
<nav className="topbar topbar-mobile">
|
<nav className="topbar topbar-mobile">
|
||||||
|
<PublishWarnings warnings={publishWarnings} onDismiss={() => setPublishWarnings([])} />
|
||||||
<div className="topbar-left">
|
<div className="topbar-left">
|
||||||
{isWHP && (
|
{isWHP && (
|
||||||
<a
|
<a
|
||||||
@@ -307,6 +315,7 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<nav className="topbar">
|
<nav className="topbar">
|
||||||
|
<PublishWarnings warnings={publishWarnings} onDismiss={() => setPublishWarnings([])} />
|
||||||
<div className="topbar-left">
|
<div className="topbar-left">
|
||||||
{isWHP && (
|
{isWHP && (
|
||||||
<a href={whpConfig!.backUrl} className="topbar-btn back-btn" aria-label="Back to Panel">
|
<a href={whpConfig!.backUrl} className="topbar-btn back-btn" aria-label="Back to Panel">
|
||||||
|
|||||||
@@ -106,6 +106,11 @@ body {
|
|||||||
border-bottom: 1px solid var(--color-border);
|
border-bottom: 1px solid var(--color-border);
|
||||||
z-index: 100;
|
z-index: 100;
|
||||||
gap: 12px;
|
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,
|
.topbar-left,
|
||||||
@@ -1884,3 +1889,33 @@ body {
|
|||||||
height: 44px !important;
|
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;
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user