diff --git a/frontend/src/admin/EmailTemplateEditor.tsx b/frontend/src/admin/EmailTemplateEditor.tsx index 036977a..62da949 100644 --- a/frontend/src/admin/EmailTemplateEditor.tsx +++ b/frontend/src/admin/EmailTemplateEditor.tsx @@ -1,5 +1,6 @@ -import { useEffect, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import Input from 'antd/es/input'; +import type { InputRef } from 'antd/es/input'; import Button from 'antd/es/button'; import Space from 'antd/es/space'; import Tag from 'antd/es/tag'; @@ -41,6 +42,70 @@ export default function EmailTemplateEditor({ template, onChanged }: Props) { const [preview, setPreview] = useState<{ subject: string; html: string } | null>(null); const [previewError, setPreviewError] = useState(null); + /** + * Which field a placeholder chip inserts into. + * + * Clicking a chip blurs whichever field had focus, so the choice has to be + * remembered rather than read at click time. It starts on the body because + * that is where placeholders almost always go, and because landing on the tab + * and clicking a chip should do something predictable rather than nothing. + */ + const [lastFocused, setLastFocused] = useState<'subject' | 'body'>('body'); + const subjectRef = useRef(null); + // The textarea is queried from the wrapper rather than taken from MDEditor's + // ref, which exposes an internal store whose shape is not part of its API. + // The editor renders exactly one textarea. + const bodyWrapRef = useRef(null); + /** + * Where to put the caret once React has re-rendered with the new value. + * + * Both fields are controlled, so an insert is a string splice plus a state + * update, after which the caret would otherwise jump to the end. A ref rather + * than state: this is a message to the effect below, not something render + * depends on. + */ + const pendingCaret = useRef<{ field: 'subject' | 'body'; at: number } | null>(null); + + function fieldElement(field: 'subject' | 'body'): HTMLInputElement | HTMLTextAreaElement | null { + if (field === 'subject') return subjectRef.current?.input ?? null; + return bodyWrapRef.current?.querySelector('textarea') ?? null; + } + + /** + * Inserts a placeholder at the caret, replacing any selection. + * + * Goes through the same setState the keyboard does. Writing into the + * element's `value` directly would appear to work and would not: the next + * keystroke re-renders from state and the insert vanishes. + */ + function insertPlaceholder(name: string) { + const tag = `{{${name}}}`; + const field = lastFocused; + const element = fieldElement(field); + const current = field === 'subject' ? subject : body; + // Falls back to the end of the text when the field has never been focused, + // so a chip clicked immediately still does something sensible. + const start = element?.selectionStart ?? current.length; + const end = element?.selectionEnd ?? current.length; + + const next = current.slice(0, start) + tag + current.slice(end); + if (field === 'subject') setSubject(next); + else setBody(next); + pendingCaret.current = { field, at: start + tag.length }; + } + + // Runs after the value has landed, so the caret sits just past what was + // inserted and typing carries on from there. + useEffect(() => { + const pending = pendingCaret.current; + if (!pending) return; + pendingCaret.current = null; + const element = fieldElement(pending.field); + if (!element) return; + element.focus(); + element.setSelectionRange(pending.at, pending.at); + }, [subject, body]); + const customised = template.subject !== null || template.body !== null; // Previews the draft in the editor, not what is stored, so the effect of an @@ -103,16 +168,32 @@ export default function EmailTemplateEditor({ template, onChanged }: Props) { . + {/* Real buttons, not decorated spans. An antd Tag renders a span, so + before this a keyboard user could neither reach a chip nor activate + one. The button carries the semantics and the Tag the appearance, so + enter and space work without any key handling of our own. */} {template.available.map((name) => ( - {`{{${name}}}`} + ))} setSubject(e.target.value)} + onFocus={() => setLastFocused('subject')} placeholder="Subject" style={{ marginBottom: 8 }} /> @@ -125,7 +206,7 @@ export default function EmailTemplateEditor({ template, onChanged }: Props) { a literal {{resetUrl}} and no consent footer. The pane on the right is the server's rendering of the actual email, and two previews disagreeing about the thing being edited is worse than one. */} -
+
setBody(val || '')} @@ -135,7 +216,10 @@ export default function EmailTemplateEditor({ template, onChanged }: Props) { // textarea, so the label has to be passed through — without it every // email template test loses its handle on the field, and a screen // reader loses the only thing naming it. - textareaProps={{ 'aria-label': `${template.label} body` }} + textareaProps={{ + 'aria-label': `${template.label} body`, + onFocus: () => setLastFocused('body') + }} />
diff --git a/frontend/tests/e2e/pages/AdminEmails.ts b/frontend/tests/e2e/pages/AdminEmails.ts index a032feb..6bb62c7 100644 --- a/frontend/tests/e2e/pages/AdminEmails.ts +++ b/frontend/tests/e2e/pages/AdminEmails.ts @@ -20,6 +20,16 @@ export class AdminEmails { this.restoreDefaultButton = page.getByRole('button', { name: 'Restore default' }); } + /** + * A placeholder chip, which inserts its tag at the caret (#143). + * + * Located by the accessible name rather than the visible text, because the + * visible text is `{{name}}` and the braces make an awkward locator. + */ + placeholderChip(name: string): Locator { + return this.page.getByRole('button', { name: `Insert {{${name}}}` }); + } + /** One template's entry in the rail. */ railTab(label: string | RegExp): Locator { return this.page.getByRole('tab', { name: label }); diff --git a/frontend/tests/e2e/placeholder-chips.spec.ts b/frontend/tests/e2e/placeholder-chips.spec.ts new file mode 100644 index 0000000..e3c7fdb --- /dev/null +++ b/frontend/tests/e2e/placeholder-chips.spec.ts @@ -0,0 +1,119 @@ +import { test, expect } from './fixtures'; + +// Uses the verification template, not the password reset one. Stored templates +// live in admin_settings and are global per database, so two spec files editing +// the same key race each other across Playwright's workers — which is exactly +// what happened when this was first written against passwordReset, the key +// email-templates.spec.ts already owns. Different key, no race. +// +// Each test still restores what it touched, so a later spec reads the built-in +// copy rather than whatever this left behind. +async function restore(request: import('@playwright/test').APIRequestContext, key: string) { + await request.delete(`/api/admin/email-templates/${key}`); +} + +const LABEL = 'Email verification'; + +test.describe.configure({ mode: 'serial' }); + +test.describe('Inserting placeholders from the chips', () => { + test.afterEach(async ({ page }) => { + await restore(page.request, 'verification'); + }); + + test('inserts into the body at the caret, not at the end', async ({ admin, adminEmails }) => { + await admin.open('Emails'); + await adminEmails.openTemplate(LABEL); + + const body = adminEmails.body(LABEL); + await body.fill('BEFORE AFTER'); + // Caret between the two words. Appending would put the tag after AFTER, + // which is the failure this test exists to catch. + await body.click(); + await body.press('Control+Home'); + for (let i = 0; i < 'BEFORE '.length; i++) await body.press('ArrowRight'); + + await adminEmails.placeholderChip('greeting').click(); + + await expect(body).toHaveValue('BEFORE {{greeting}}AFTER'); + }); + + test('leaves the caret after the tag, so typing continues from there', async ({ + admin, + adminEmails + }) => { + await admin.open('Emails'); + await adminEmails.openTemplate(LABEL); + + const body = adminEmails.body(LABEL); + await body.fill(''); + await body.click(); + + await adminEmails.placeholderChip('greeting').click(); + // No click in between: the field must still hold focus with the caret + // placed just past what was inserted. + await body.pressSequentially('!'); + + await expect(body).toHaveValue('{{greeting}}!'); + }); + + test('replaces a selection rather than inserting alongside it', async ({ + admin, + adminEmails + }) => { + await admin.open('Emails'); + await adminEmails.openTemplate(LABEL); + + const body = adminEmails.body(LABEL); + await body.fill('REPLACE ME'); + await body.click(); + await body.press('Control+a'); + + await adminEmails.placeholderChip('verifyUrl').click(); + + await expect(body).toHaveValue('{{verifyUrl}}'); + }); + + test('inserts into the subject when that was the field last used', async ({ + admin, + adminEmails + }) => { + await admin.open('Emails'); + await adminEmails.openTemplate(LABEL); + + // Captured first: this template's default body already contains + // {{greeting}}, so "the body does not contain the tag" would be false + // before the click as well as after. Unchanged is the real claim. + const bodyBefore = await adminEmails.body(LABEL).inputValue(); + + const subject = adminEmails.subject(LABEL); + await subject.fill('Hi there'); + await subject.click(); + await subject.press('Control+Home'); + for (let i = 0; i < 'Hi '.length; i++) await subject.press('ArrowRight'); + + await adminEmails.placeholderChip('greeting').click(); + + await expect(subject).toHaveValue('Hi {{greeting}} there'); + await expect(adminEmails.body(LABEL)).toHaveValue(bodyBefore); + }); + + test('is reachable and operable from the keyboard', async ({ page, admin, adminEmails }) => { + await admin.open('Emails'); + await adminEmails.openTemplate(LABEL); + + await adminEmails.body(LABEL).fill(''); + await adminEmails.body(LABEL).click(); + + // Focused directly rather than tabbed to, since the number of tab stops + // before it is a layout detail. What matters is that a button can take + // focus at all and responds to a key — an antd Tag renders a span and + // could do neither. + const chip = adminEmails.placeholderChip('greeting'); + await chip.focus(); + await expect(chip).toBeFocused(); + await page.keyboard.press('Enter'); + + await expect(adminEmails.body(LABEL)).toHaveValue('{{greeting}}'); + }); +});