From d7eeff3a6800f36e721a9d7290df8a644a09af34 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 9 Aug 2026 11:29:26 -0700 Subject: [PATCH] feat(site-builder): add in-builder Report an Issue modal Wires the Task 18 payload builder and Task 16/17 diagnostics into a user-facing modal, reachable from the topbar bug icon (desktop) and overflow menu (mobile). Two gaps in the task brief's draft, not called out there, are handled explicitly: buildReportPayload() can throw when the payload is still oversized after canvas_state is dropped, so submission is wrapped in try/catch with an actionable "too large" error that preserves the user's typed text; and the textarea maxLength is sourced from MAX_DESCRIPTION_CHARS (with a proximity character count) instead of a hardcoded number, so the UI limit can't drift from the payload limit. Also portals the modal to document.body for the same stacking-context reasons TemplateModal/HeadCodeModal already do, and guards the useEditor() selection collector with optional chaining so it degrades gracefully under TopBar's existing test harness (a minimal @craftjs/core stub with no events/nodes on its collector state). The textarea uses onInput rather than onChange: React's onChange dedup (via its DOM value-tracker) treats a test harness's raw `el.value = x` assignment as a no-op change, matching this repo's existing pattern (HeadCodeModal.test.tsx, shared-controls.test.tsx, etc. all work around the same gotcha) -- onInput is a plain passthrough with no such check, and is behaviorally identical for real typing. Co-Authored-By: Claude Opus 5 (1M context) --- .../panels/topbar/ReportIssueModal.test.tsx | 177 ++++++++++ craft/src/panels/topbar/ReportIssueModal.tsx | 308 ++++++++++++++++++ craft/src/panels/topbar/TopBar.tsx | 14 + .../src/panels/topbar/TopBarOverflowMenu.tsx | 5 + 4 files changed, 504 insertions(+) create mode 100644 craft/src/panels/topbar/ReportIssueModal.test.tsx create mode 100644 craft/src/panels/topbar/ReportIssueModal.tsx diff --git a/craft/src/panels/topbar/ReportIssueModal.test.tsx b/craft/src/panels/topbar/ReportIssueModal.test.tsx new file mode 100644 index 0000000..77c8527 --- /dev/null +++ b/craft/src/panels/topbar/ReportIssueModal.test.tsx @@ -0,0 +1,177 @@ +import { describe, test, expect, vi, beforeEach } from 'vitest'; +import React from 'react'; +import { createRoot, Root } from 'react-dom/client'; +import { act } from 'react-dom/test-utils'; + +vi.mock('@craftjs/core', () => ({ + useEditor: (collect?: (state: any) => any) => { + const state = { events: { selected: new Set() }, nodes: {} }; + return { + query: { serialize: () => '{"ROOT":{}}' }, + actions: {}, + ...(collect ? collect(state) : {}), + }; + }, +})); +vi.mock('../../state/EditorConfigContext', () => ({ + useEditorConfig: () => ({ + whpConfig: { + apiUrl: '/panel/api/site-builder', + csrfToken: 'tok', + siteId: 42, + siteDomain: 'example.com', + }, + isWHP: true, + }), +})); +vi.mock('../../state/PageContext', () => ({ + usePages: () => ({ + activePageId: 'home', + pages: [{ id: 'home', name: 'Home', slug: 'index', craftState: null }], + }), +})); +// Mocked (rather than using the real ring buffer) only so the added +// oversized-payload test below can force an over-cap consoleErrors array -- +// every other test gets a plain empty array, same as a fresh page load. +vi.mock('../../utils/console-buffer', () => ({ getRecentConsoleErrors: vi.fn() })); + +import { ReportIssueModal } from './ReportIssueModal'; +import { getRecentConsoleErrors } from '../../utils/console-buffer'; + +let container: HTMLDivElement; +let root: Root; + +function render(open = true) { + container = document.createElement('div'); + document.body.appendChild(container); + act(() => { + root = createRoot(container); + root.render(); + }); +} + +function typeDescription(text: string) { + const ta = document.querySelector('[data-testid="report-description"]') as HTMLTextAreaElement; + act(() => { + ta.value = text; + ta.dispatchEvent(new Event('input', { bubbles: true })); + }); +} + +function submit() { + const btn = document.querySelector('[data-action="submit-report"]') as HTMLButtonElement; + act(() => { btn.click(); }); +} + +beforeEach(() => { + vi.restoreAllMocks(); + document.body.innerHTML = ''; + vi.mocked(getRecentConsoleErrors).mockReturnValue([]); +}); + +describe('ReportIssueModal', () => { + test('submit is disabled until a description is entered', () => { + render(); + expect((document.querySelector('[data-action="submit-report"]') as HTMLButtonElement).disabled).toBe(true); + typeDescription('something is wrong'); + expect((document.querySelector('[data-action="submit-report"]') as HTMLButtonElement).disabled).toBe(false); + }); + + test('posts the payload to the report_issue action with the CSRF header', async () => { + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ success: true, reference: 'SB-1234', id: 1234 }), + }); + vi.stubGlobal('fetch', fetchMock); + + render(); + typeDescription('colours do nothing'); + await act(async () => { submit(); }); + + expect(fetchMock).toHaveBeenCalledTimes(1); + const [url, init] = fetchMock.mock.calls[0]; + expect(url).toContain('action=report_issue'); + expect(init.method).toBe('POST'); + expect(init.headers['X-CSRF-Token']).toBe('tok'); + + const body = JSON.parse(init.body); + expect(body.description).toBe('colours do nothing'); + expect(body.category).toBe('bug'); + expect(body.site_id).toBe(42); + expect(body.canvas_state).toBe('{"ROOT":{}}'); + }); + + test('unchecking include-contents omits the canvas state', async () => { + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ success: true, reference: 'SB-2', id: 2 }), + }); + vi.stubGlobal('fetch', fetchMock); + + render(); + typeDescription('no canvas please'); + const cb = document.querySelector('[data-testid="report-include-canvas"]') as HTMLInputElement; + act(() => { cb.click(); }); + await act(async () => { submit(); }); + + const body = JSON.parse(fetchMock.mock.calls[0][1].body); + expect(body.canvas_state).toBeNull(); + expect(body.canvas_state_omitted).toBe('opt-out'); + }); + + test('shows the returned reference on success', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ success: true, reference: 'SB-1234', id: 1234 }), + })); + render(); + typeDescription('x'); + await act(async () => { submit(); }); + expect(document.body.textContent).toContain('SB-1234'); + }); + + test('keeps the text and shows an error when the request fails', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: false, + json: async () => ({ success: false, error: 'Rate limited' }), + })); + render(); + typeDescription('keep me'); + await act(async () => { submit(); }); + + expect(document.body.textContent).toContain('Rate limited'); + expect((document.querySelector('[data-testid="report-description"]') as HTMLTextAreaElement).value).toBe('keep me'); + }); + + test('a payload still too large after dropping canvas_state shows an error instead of hanging, and never calls fetch', async () => { + // buildReportPayload throws when the body is still over MAX_PAYLOAD_BYTES + // even with canvas_state dropped -- see report-payload.ts `finalize()`. + // A huge *description* alone can't reach that path (it's truncated to + // MAX_DESCRIPTION_CHARS before the size check ever runs -- see + // report-payload.test.ts's "description size bounding" suite), so this + // forces the same defensive scenario report-payload.test.ts's "honest + // size markers" test uses: an oversized consoleErrors array, standing in + // for whatever future bug would let that much data through in practice. + // The point under test here is purely the modal's reaction to the throw, + // not how the oversized condition arises. + vi.mocked(getRecentConsoleErrors).mockReturnValue( + Array.from({ length: 5000 }, (_, i) => ({ ts: i, message: 'x'.repeat(200) })), + ); + const fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + + render(); + typeDescription('this report has a huge console-error backlog attached'); + await act(async () => { submit(); }); + + expect(fetchMock).not.toHaveBeenCalled(); + expect(document.body.textContent).toMatch(/too large/i); + // The user's text must survive a rejected submission just as much as a + // server-side failure does. + expect((document.querySelector('[data-testid="report-description"]') as HTMLTextAreaElement).value).toBe( + 'this report has a huge console-error backlog attached', + ); + // Not stuck on "Sending..." -- the submit button is usable again. + expect((document.querySelector('[data-action="submit-report"]') as HTMLButtonElement).disabled).toBe(false); + }); +}); diff --git a/craft/src/panels/topbar/ReportIssueModal.tsx b/craft/src/panels/topbar/ReportIssueModal.tsx new file mode 100644 index 0000000..557f9bc --- /dev/null +++ b/craft/src/panels/topbar/ReportIssueModal.tsx @@ -0,0 +1,308 @@ +import React, { useState } from 'react'; +import { createPortal } from 'react-dom'; +import { useEditor } from '@craftjs/core'; +import { Modal } from '../../ui/Modal'; +import { useEditorConfig } from '../../state/EditorConfigContext'; +import { usePages } from '../../state/PageContext'; +import { buildReportPayload, MAX_DESCRIPTION_CHARS, type ReportCategory } from '../../utils/report-payload'; +import { getRecentConsoleErrors } from '../../utils/console-buffer'; +import { editorBuild } from '../../utils/build-stamp'; + +export interface ReportIssueModalProps { + open: boolean; + onClose: () => void; + device: string; +} + +const CATEGORIES: { value: ReportCategory; label: string }[] = [ + { value: 'bug', label: 'Something is broken' }, + { value: 'confusing', label: 'Something is confusing' }, + { value: 'feature', label: 'I wish it could…' }, +]; + +/** Only start showing the running character count once it's actually useful + * -- i.e. once the user is close enough to MAX_DESCRIPTION_CHARS that + * losing text is a real possibility, not on every keystroke from zero. */ +const COUNTER_THRESHOLD = MAX_DESCRIPTION_CHARS - 500; + +export const ReportIssueModal: React.FC = ({ open, onClose, device }) => { + const { whpConfig } = useEditorConfig(); + const { activePageId, pages } = usePages(); + // Guarded with optional chaining: some hosts around this component (e.g. + // TopBar's own test harness) stub `useEditor` with a minimal collector + // state that has no `events`/`nodes` at all -- this must degrade to "no + // selection known" rather than throw and take the whole topbar down. + const { query, selectedType } = useEditor((state: any) => { + const sel = state?.events?.selected; + const id = sel && sel.size > 0 ? (Array.from(sel)[0] as string) : null; + return { selectedType: id ? (state?.nodes?.[id]?.data?.displayName ?? null) : null }; + }); + + const [category, setCategory] = useState('bug'); + const [description, setDescription] = useState(''); + const [includeCanvas, setIncludeCanvas] = useState(true); + const [status, setStatus] = useState<'idle' | 'sending' | 'sent' | 'error'>('idle'); + const [reference, setReference] = useState(''); + const [error, setError] = useState(''); + + const activePage = pages.find((p) => p.id === activePageId); + + const reset = (): void => { + setDescription(''); + setStatus('idle'); + setReference(''); + setError(''); + }; + + const handleSubmit = async (): Promise => { + if (!description.trim() || !whpConfig) return; + setStatus('sending'); + setError(''); + + let canvasState: string | null = null; + try { + canvasState = query.serialize(); + } catch { + // A serialize failure must not block the report -- it is often the + // very thing being reported. + canvasState = null; + } + + let payload; + try { + payload = buildReportPayload({ + category, + description, + includeCanvas, + siteId: whpConfig.siteId ?? null, + siteDomain: whpConfig.siteDomain ?? '', + pageId: activePageId, + pageSlug: activePage?.slug ?? '', + editorVersion: editorBuild(), + userAgent: typeof navigator !== 'undefined' ? navigator.userAgent : '', + viewport: typeof window !== 'undefined' ? `${window.innerWidth}x${window.innerHeight}` : '', + deviceMode: device, + selectedType, + consoleErrors: getRecentConsoleErrors(), + canvasState, + }); + } catch { + // buildReportPayload throws (rather than returning an oversized body) + // when the payload is still over the cap even after dropping + // canvas_state -- e.g. an enormous description. The thrown value is a + // plain Error with no discriminator, so treat ANY throw here as "too + // large" rather than string-matching the message. The user's text is + // left untouched in the textarea (state.description is never reset on + // this path) so nothing is lost -- they just need to shorten it or + // untick "include this page's contents". + setError( + 'This report is too large to send, even without the page contents. ' + + 'Try unchecking "Include this page\'s contents" below, or shortening your description.', + ); + setStatus('error'); + return; + } + + try { + const resp = await fetch(`${whpConfig.apiUrl}?action=report_issue`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-CSRF-Token': whpConfig.csrfToken, + }, + body: JSON.stringify(payload), + }); + const data = await resp.json(); + if (!resp.ok || !data.success) { + setError(data.error || 'Could not send the report. Please try again.'); + setStatus('error'); + return; + } + setReference(data.reference || `SB-${data.id}`); + setStatus('sent'); + } catch (e) { + setError('Could not reach the server. Your text is still here — try again.'); + setStatus('error'); + } + }; + + const handleClose = (): void => { + if (status === 'sent') reset(); + onClose(); + }; + + return createPortal( + +
e.stopPropagation()} + > +
+
Report an issue
+ +
+ + {status === 'sent' ? ( +
+