From bdde1c8b5ca7f844e6b8e8ddda11d071eca6dee0 Mon Sep 17 00:00:00 2001 From: William Laugesen Date: Wed, 19 Aug 2026 17:03:00 +1200 Subject: [PATCH 1/2] Require a comment before the feedback form will send Send stays disabled until the box has text in it. Every junk row in the responses arrived with an empty comment, so this drops all of them. Co-Authored-By: Claude Opus 5 (1M context) --- src/components/Feedback.astro | 1 + src/scripts/modules/feedback.js | 24 ++++++++++++++++++---- tests/feedback.spec.ts | 36 +++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/components/Feedback.astro b/src/components/Feedback.astro index 53d1db839b..fd4a57aa50 100644 --- a/src/components/Feedback.astro +++ b/src/components/Feedback.astro @@ -62,6 +62,7 @@ stats.stop(); label={_(Translations.octopus_feedback.send)} size="small" importance="loud" + disabled data-feedback-send /> diff --git a/src/scripts/modules/feedback.js b/src/scripts/modules/feedback.js index dfdff67c63..55104c7800 100644 --- a/src/scripts/modules/feedback.js +++ b/src/scripts/modules/feedback.js @@ -23,9 +23,9 @@ async function submit(page, rating, comment) { const body = new URLSearchParams(); body.set(FIELD_PAGE, page); body.set(FIELD_RATING, rating); - // The comment is marked required on the form, so a blank box still has to - // send something for the submission to be accepted at all. - body.set(FIELD_COMMENT, comment.trim() || ' '); + // Required on the form, and required by the widget as well, so there is + // always something here. + body.set(FIELD_COMMENT, comment.trim()); await fetch(FORM_URL, { method: 'POST', mode: 'no-cors', body }); } @@ -65,9 +65,23 @@ class Feedback { this.votes.forEach((button) => { button.addEventListener('click', () => this.vote(button)); }); + this.textarea.addEventListener('input', () => this.allowSend()); this.send.addEventListener('click', () => this.submit()); } + /** + * The security scanner that crawls the docs works every control it finds and + * types into none of them, so each of its submissions arrives with an empty + * box. Send is out of reach until there is something worth sending. + */ + allowSend() { + if (this.textarea.value.trim()) { + this.send.removeAttribute('disabled'); + } else { + this.send.setAttribute('disabled', ''); + } + } + /** @param {HTMLElement} chosen */ vote(chosen) { this.rating = chosen.dataset.feedbackVote ?? null; @@ -78,7 +92,9 @@ class Feedback { } async submit() { - if (!this.rating) return; + // The disabled button covers both of these already. They are here for the + // caller that reaches the handler another way. + if (!this.rating || !this.textarea.value.trim()) return; // Guards against a second submission while the first is in flight. this.send.setAttribute('disabled', ''); diff --git a/tests/feedback.spec.ts b/tests/feedback.spec.ts index 7a80a2c69d..b8ec26519f 100644 --- a/tests/feedback.spec.ts +++ b/tests/feedback.spec.ts @@ -47,6 +47,38 @@ test.describe('feedback widget', () => { ).toHaveAttribute('aria-pressed', 'true'); }); + test('keeps send out of reach until the box has something in it', async ({ + page, + }) => { + await page.locator(`[data-feedback-vote="${RATING_YES}"]`).click(); + await expect(page.locator('[data-feedback-send]')).toBeDisabled(); + + // Whitespace is nothing to send, so it does not count as an answer. + await page.locator('.feedback__textarea').fill(' '); + await expect(page.locator('[data-feedback-send]')).toBeDisabled(); + + await page.locator('.feedback__textarea').fill('the diagram is wrong'); + await expect(page.locator('[data-feedback-send]')).toBeEnabled(); + + await page.locator('.feedback__textarea').fill(''); + await expect(page.locator('[data-feedback-send]')).toBeDisabled(); + }); + + test('sends nothing on a vote with an empty box', async ({ page }) => { + let sent = false; + await page.route('**/docs.google.com/**', (route) => { + sent = true; + return route.fulfill({ status: 200, body: '' }); + }); + + await page.locator(`[data-feedback-vote="${RATING_YES}"]`).click(); + // Past the disabled attribute, which a click alone cannot get through. + await page.locator('[data-feedback-send]').dispatchEvent('click'); + + await expect(page.locator('.feedback__thanks')).toBeHidden(); + expect(sent).toBe(false); + }); + test('thanks the reader once the submission has gone out', async ({ page, }) => { @@ -55,6 +87,7 @@ test.describe('feedback widget', () => { ); await page.locator(`[data-feedback-vote="${RATING_YES}"]`).click(); + await page.locator('.feedback__textarea').fill('clear enough'); await page.locator('[data-feedback-send]').click(); await expect(page.locator('.feedback__thanks')).toBeVisible(); @@ -89,6 +122,9 @@ test.describe('feedback widget', () => { page, }) => { await page.locator(`[data-feedback-vote="${RATING_NO}"]`).click(); + await page + .locator('.feedback__textarea') + .fill('the steps are out of order'); await page.locator('[data-feedback-send]').click(); // Send is disabled for the attempt and only comes back on the failure, so From 74104529758bc1df2b8fb3582070d55d116bd8e2 Mon Sep 17 00:00:00 2001 From: William Laugesen Date: Thu, 20 Aug 2026 08:34:09 +1200 Subject: [PATCH 2/2] Mark the feedback comment as required in its label Send stays disabled until the box has text, so the label saying optional contradicted the widget. Co-Authored-By: Claude Opus 5 (1M context) --- src/data/language.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/data/language.json b/src/data/language.json index 3b994a4592..15ff38e75f 100644 --- a/src/data/language.json +++ b/src/data/language.json @@ -31,7 +31,7 @@ "en": "No" }, "comment_label": { - "en": "Why did you give this rating? (optional)" + "en": "Why did you give this rating? (required)" }, "send": { "en": "Send"