Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/components/Feedback.astro
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ stats.stop();
label={_(Translations.octopus_feedback.send)}
size="small"
importance="loud"
disabled
data-feedback-send
/>
</div>
Expand Down
2 changes: 1 addition & 1 deletion src/data/language.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
24 changes: 20 additions & 4 deletions src/scripts/modules/feedback.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
Expand Down Expand Up @@ -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;
Expand All @@ -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', '');
Expand Down
36 changes: 36 additions & 0 deletions tests/feedback.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}) => {
Expand All @@ -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();
Expand Down Expand Up @@ -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
Expand Down