diff --git a/data/ui.yml b/data/ui.yml index 17ef3dcdc4..e61cbbdad9 100644 --- a/data/ui.yml +++ b/data/ui.yml @@ -65,9 +65,11 @@ survey: email_label: If we can contact you with more questions, please enter your email address email_validation: Please enter a valid email address send: Send + next: Next feedback: Thank you! We received your feedback. - not_support: If you need a reply, please contact support instead. + not_support: If you need a reply, please contact support. privacy_policy: Privacy policy + server_error: Unable to process comment at the moment. Please try again. contribution_cta: title: Help us make these docs great! body: All GitHub docs are open source. See something that's wrong or unclear? Submit a pull request. diff --git a/src/events/components/Survey.tsx b/src/events/components/Survey.tsx index aee8596dc9..16a1fed73f 100644 --- a/src/events/components/Survey.tsx +++ b/src/events/components/Survey.tsx @@ -2,6 +2,9 @@ import { useState, useRef, useEffect } from 'react' import cx from 'classnames' import { useRouter } from 'next/router' import { ThumbsdownIcon, ThumbsupIcon } from '@primer/octicons-react' +import { Spinner } from '@primer/react' +import useSWR from 'swr' + import { useTranslation } from 'src/languages/components/useTranslation' import { Link } from 'src/frame/components/Link' import { sendEvent, EventType, startVisitTime } from 'src/events/components/events' @@ -10,23 +13,30 @@ import styles from './Survey.module.scss' enum ViewState { START = 'START', + NEXT = 'NEXT', + END = 'END', +} + +enum VoteState { YES = 'YES', NO = 'NO', - END = 'END', } export const Survey = () => { const { asPath, locale } = useRouter() const { t } = useTranslation('survey') const [state, setState] = useState(ViewState.START) + const [voteState, setVoteState] = useState(null) const [isEmailError, setIsEmailError] = useState(false) const formRef = useRef(null) + const [comment, setComment] = useState('') useEffect(() => { // Always reset the form if navigating to a new page because what // you might have said or started to say belongs exclusively to // to the page you started on. setState(ViewState.START) + setVoteState(null) }, [asPath]) useEffect(() => { @@ -42,10 +52,10 @@ export const Survey = () => { } }, [state]) - function vote(state: ViewState) { + function vote(vote: VoteState) { return () => { trackEvent(getFormData()) - setState(state) + setVoteState(vote) } } @@ -64,12 +74,43 @@ export const Survey = () => { } } + const { data, error, isLoading } = useSWR( + state === ViewState.NEXT && comment.trim() ? '/api/events/survey/preview/v1' : null, + async (url: string) => { + const response = await fetch(url, { + method: 'POST', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ + comment: comment.trim(), + locale, + url: `/${locale}${asPath}`, + vote: voteState, + }), + }) + if (response.ok) { + return await response.json() + } else { + throw new Error(`${response.status} on preview`) + } + }, + ) + + const hasPreview = !!data && !error + function submit(evt: React.FormEvent) { evt.preventDefault() - trackEvent(getFormData()) - if (!isEmailError) { - setState(ViewState.END) - setIsEmailError(false) + if (hasPreview) { + trackEvent(getFormData()) + if (!isEmailError) { + setState(ViewState.END) + setIsEmailError(false) + setComment('') + } + } else if (comment.trim()) { + setState(ViewState.NEXT) } } @@ -100,18 +141,21 @@ export const Survey = () => { name="survey-vote" value="Y" aria-label={t`yes`} - onChange={vote(ViewState.YES)} - checked={state === ViewState.YES} + onChange={vote(VoteState.YES)} + checked={voteState === VoteState.YES} /> { name="survey-vote" value="N" aria-label={t`no`} - onChange={vote(ViewState.NO)} - checked={state === ViewState.NO} + onChange={vote(VoteState.NO)} + checked={voteState === VoteState.NO} /> )} - {[ViewState.YES, ViewState.NO].includes(state) && ( + {error && {t`server_error`}} + + {[ViewState.START, ViewState.NEXT].includes(state) && voteState && ( <>

@@ -150,48 +199,75 @@ export const Survey = () => { className="form-control input-sm width-full" name="survey-comment" id="survey-comment" + value={comment} + onChange={(event) => setComment(event.target.value)} >

-
- - - {isEmailError && ( -

- {t`email_validation`} -

- )} -
- {t`not_support`} + {hasPreview && ( +
+ + + {isEmailError && ( +

+ {t`email_validation`} +

+ )} +
+ )} + + {hasPreview && ( + + )}
- + {hasPreview ? ( + + ) : ( + + )}
)} diff --git a/src/events/middleware.js b/src/events/middleware.js index e70d6bd951..5d31b12b09 100644 --- a/src/events/middleware.js +++ b/src/events/middleware.js @@ -77,4 +77,32 @@ router.post( }), ) +router.post( + '/survey/preview/v1', + catchMiddlewareError(async function previewComment(req, res) { + noCacheControl(res) + + const { comment, locale, url, vote } = req.body + + console.log(`The comment was posted in ${locale} on ${url} with vote ${vote}`) + + if (!comment || !comment.trim()) { + return res.status(400).json({ message: 'Empty comment' }) + } + + const signals = [] + const rating = 1.0 + + // if (comment.includes('@') && !comment.includes(' ')) { + // // XXX Make it a simple email validator + // signals.push({ + // email: 'Looks like an email address', + // }) + // rating -= 0.1 + // } + + return res.json({ rating, signals }) + }), +) + export default router diff --git a/src/events/tests/middleware.js b/src/events/tests/middleware.js index 82cddf9e04..4d65542d02 100644 --- a/src/events/tests/middleware.js +++ b/src/events/tests/middleware.js @@ -113,3 +113,57 @@ describe('POST /events', () => { expect(statusCode).toBe(400) }) }) + +// These are mostly placeholder tests for now since most of the +// implementation of this endpoint is not yet written. +describe('POST /events/survey/preview/v1', () => { + test('should repond with 400 when no comment is provided', async () => { + const body = JSON.stringify({ + locale: 'en', + url: '/quickstart', + vote: 'yes', + }) + const res = await post('/api/events/survey/preview/v1', { + body, + headers: { + 'content-type': 'application/json', + }, + }) + expect(res.statusCode).toBe(400) + }) + + test('should repond with 400 when comment is provided but empty', async () => { + const body = JSON.stringify({ + locale: 'en', + url: '/quickstart', + vote: 'yes', + comment: ' ', + }) + const res = await post('/api/events/survey/preview/v1', { + body, + headers: { + 'content-type': 'application/json', + }, + }) + expect(res.statusCode).toBe(400) + }) + + test('should repond with 200 when comment is provided', async () => { + const body = JSON.stringify({ + locale: 'en', + url: '/quickstart', + vote: 'yes', + comment: 'Wonderful', + }) + const res = await post('/api/events/survey/preview/v1', { + body, + headers: { + 'content-type': 'application/json', + }, + }) + const respBody = JSON.parse(res.body) + expect(res.statusCode).toBe(200) + expect(respBody.rating).toEqual(1.0) + expect(respBody.signals).toEqual([]) + }) +}) diff --git a/src/fixtures/fixtures/data/ui.yml b/src/fixtures/fixtures/data/ui.yml index 17ef3dcdc4..e61cbbdad9 100644 --- a/src/fixtures/fixtures/data/ui.yml +++ b/src/fixtures/fixtures/data/ui.yml @@ -65,9 +65,11 @@ survey: email_label: If we can contact you with more questions, please enter your email address email_validation: Please enter a valid email address send: Send + next: Next feedback: Thank you! We received your feedback. - not_support: If you need a reply, please contact support instead. + not_support: If you need a reply, please contact support. privacy_policy: Privacy policy + server_error: Unable to process comment at the moment. Please try again. contribution_cta: title: Help us make these docs great! body: All GitHub docs are open source. See something that's wrong or unclear? Submit a pull request. diff --git a/src/fixtures/tests/playwright-rendering.spec.ts b/src/fixtures/tests/playwright-rendering.spec.ts index f339a4b3b7..072e014917 100644 --- a/src/fixtures/tests/playwright-rendering.spec.ts +++ b/src/fixtures/tests/playwright-rendering.spec.ts @@ -503,9 +503,19 @@ test.describe('survey', () => { // The label is visually an SVG. Finding it by its `for` value feels easier. await page.locator('[for=survey-yes]').click() + await expect(page.getByRole('button', { name: 'Next' })).toBeVisible() + await expect(page.getByRole('button', { name: 'Cancel' })).toBeVisible() + await expect(page.getByRole('button', { name: 'Send' })).not.toBeVisible() + + await page.locator('[for=survey-comment]').click() + await page.locator('[for=survey-comment]').fill('This is a comment') + await page.getByRole('button', { name: 'Next' }).click() + await expect(page.getByRole('button', { name: 'Cancel' })).toBeVisible() + await expect(page.getByRole('button', { name: 'Send' })).toBeVisible() + await expect(page.getByRole('button', { name: 'Next' })).not.toBeVisible() + await page.getByPlaceholder('email@example.com').click() await page.getByPlaceholder('email@example.com').fill('test@example.com') - await page.getByRole('button', { name: 'Send' }).click() // One for the page view event, one for the thumbs up click, one for // the submission. @@ -533,9 +543,26 @@ test.describe('survey', () => { // One for the page view event and one for the thumbs up click expect(fulfilled).toBe(1 + 1) - await expect(page.getByRole('button', { name: 'Send' })).toBeVisible() + await expect(page.getByRole('button', { name: 'Next' })).toBeVisible() await page.getByRole('button', { name: 'Cancel' }).click() - await expect(page.getByRole('button', { name: 'Send' })).not.toBeVisible() + }) + + test('vote on one page, then go to another and it should reset', async ({ page }) => { + // Important to set this up *before* interacting with the page + // in case of possible race conditions. + await page.route('**/api/events', (route) => { + route.fulfill({}) + }) + + await page.goto('/get-started/foo/for-playwright') + + await expect(page.locator('[for=survey-comment]')).not.toBeVisible() + await page.locator('[for=survey-yes]').click() + await expect(page.getByRole('button', { name: 'Next' })).toBeVisible() + await expect(page.locator('[for=survey-comment]')).toBeVisible() + + await page.getByTestId('product-sidebar').getByLabel('Bar', { exact: true }).click() + await expect(page.locator('[for=survey-comment]')).not.toBeVisible() }) })