From 3dd6b54a35121cc3df3100dc204717323a1970f6 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 9 Aug 2026 11:39:22 -0700 Subject: [PATCH] =?UTF-8?q?fix(site-builder):=20address=20Task=2019=20revi?= =?UTF-8?q?ew=20=E2=80=94=20onChange=20convention,=20opt-out=20copy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on the Report an Issue modal: 1. Revert the description textarea from onInput back to onChange. The onInput swap sidestepped a real React value-tracker dedup gotcha (a test harness's raw `el.value = x` assignment looks like a no-op change to a controlled onChange input), but this codebase already has the correct fix for exactly that gotcha: bypass the tracker's patched setter via the native prototype descriptor, as HeadCodeModal.test.tsx, SiteDesignPanel.reset.test.tsx, shared-controls.test.tsx, and both MediaStylePanel.*.test.tsx already do. Rewrote typeDescription() in ReportIssueModal.test.tsx to use that idiom instead of bending the component to fit the test. 2. The include-contents opt-out copy said unchecking it still sends "your description, the page name and your browser details" but omitted console errors, which buildReportPayload always includes regardless of the checkbox and which can incidentally echo page content. Copy now lists console errors explicitly. Co-Authored-By: Claude Opus 5 (1M context) --- craft/src/panels/topbar/ReportIssueModal.test.tsx | 14 +++++++++++++- craft/src/panels/topbar/ReportIssueModal.tsx | 15 +++------------ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/craft/src/panels/topbar/ReportIssueModal.test.tsx b/craft/src/panels/topbar/ReportIssueModal.test.tsx index 77c8527..a1b8b92 100644 --- a/craft/src/panels/topbar/ReportIssueModal.test.tsx +++ b/craft/src/panels/topbar/ReportIssueModal.test.tsx @@ -50,10 +50,22 @@ function render(open = true) { }); } +// The textarea is a controlled input (`onChange`), so React's DOM value +// tracker patches its `value` setter to detect "did this really change". +// A plain `ta.value = text` assignment goes through that same patched +// setter, which updates the tracker's own record of "current value" as a +// side effect -- so by the time the dispatched 'input' event is handled, +// the tracker sees no difference and the synthetic onChange never fires. +// Bypassing the patched setter via the native prototype descriptor (same +// idiom as HeadCodeModal.test.tsx / SiteDesignPanel.reset.test.tsx / +// shared-controls.test.tsx / MediaStylePanel.*.test.tsx) sets the DOM value +// without touching the tracker, so the dispatched event is correctly seen +// as a real change. function typeDescription(text: string) { const ta = document.querySelector('[data-testid="report-description"]') as HTMLTextAreaElement; act(() => { - ta.value = text; + const setter = Object.getOwnPropertyDescriptor(window.HTMLTextAreaElement.prototype, 'value')!.set!; + setter.call(ta, text); ta.dispatchEvent(new Event('input', { bubbles: true })); }); } diff --git a/craft/src/panels/topbar/ReportIssueModal.tsx b/craft/src/panels/topbar/ReportIssueModal.tsx index 557f9bc..86cdb95 100644 --- a/craft/src/panels/topbar/ReportIssueModal.tsx +++ b/craft/src/panels/topbar/ReportIssueModal.tsx @@ -210,16 +210,7 @@ export const ReportIssueModal: React.FC = ({ open, onClos