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/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" 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