diff --git a/.changeset/friendly-feedback-close.md b/.changeset/friendly-feedback-close.md new file mode 100644 index 000000000..b6af99c7d --- /dev/null +++ b/.changeset/friendly-feedback-close.md @@ -0,0 +1,5 @@ +--- +"@reflag/browser-sdk": patch +--- + +Call `onAfterSubmit` once per feedback form, including when a user saves a score and closes without pressing Send. Closing after Send does not repeat the callback, and closing during a save waits for it to finish. Close-only callbacks include the latest saved score and an empty comment, never an unsubmitted comment draft. Replacing a form also closes the previous response. Applies to both automated surveys and manually requested feedback. diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index 4f63bbbef..a24200a0b 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -1186,6 +1186,7 @@ export class ReflagClient { openWithCommentVisible: options.openWithCommentVisible, onClose: options.onClose, onDismiss: options.onDismiss, + onAfterSubmit: options.onAfterSubmit, onScoreSubmit: async (data) => { const res = await this.feedback({ ...feedbackData, @@ -1199,13 +1200,10 @@ export class ReflagClient { return { feedbackId: undefined }; }, onSubmit: async (data) => { - // Default onSubmit handler await this.feedback({ ...feedbackData, ...data, }); - - options.onAfterSubmit?.(data); }, }); }, 1); diff --git a/packages/browser-sdk/src/feedback/feedback.ts b/packages/browser-sdk/src/feedback/feedback.ts index 1a68eeed8..1bc40683a 100644 --- a/packages/browser-sdk/src/feedback/feedback.ts +++ b/packages/browser-sdk/src/feedback/feedback.ts @@ -12,11 +12,7 @@ import { } from "./prompts"; import * as feedbackLib from "./ui"; import { DEFAULT_POSITION } from "./ui"; -import { - FeedbackSubmission, - FeedbackTranslations, - OpenFeedbackFormOptions, -} from "./ui/types"; +import { FeedbackTranslations, OpenFeedbackFormOptions } from "./ui/types"; const INITIAL_FETCH_RETRY_DELAYS_MS = [0, 5000]; @@ -59,17 +55,6 @@ export type RequestFeedbackData = Omit< */ companyId?: string; - /** - * Allows you to handle a copy of the already submitted - * feedback. - * - * This can be used for side effects, such as storing a - * copy of the feedback in your own application or CRM. - * - * @param {Object} data - */ - onAfterSubmit?: (data: FeedbackSubmission) => void; - /** * Flag key. */ @@ -436,7 +421,6 @@ export class AutoFeedback { }, onSubmit: async (data) => { await replyCallback(data); - options.onAfterSubmit?.(data); }, onDismiss: () => replyCallback(null), position: this.position, diff --git a/packages/browser-sdk/src/feedback/ui/FeedbackDialog.tsx b/packages/browser-sdk/src/feedback/ui/FeedbackDialog.tsx index d5a834064..9f02fedfb 100644 --- a/packages/browser-sdk/src/feedback/ui/FeedbackDialog.tsx +++ b/packages/browser-sdk/src/feedback/ui/FeedbackDialog.tsx @@ -1,5 +1,5 @@ import { Fragment, FunctionComponent, h } from "preact"; -import { useCallback, useState } from "preact/hooks"; +import { useCallback, useLayoutEffect, useRef, useState } from "preact/hooks"; import { feedbackContainerId } from "../../ui/constants"; import { Dialog, useDialog } from "../../ui/Dialog"; @@ -40,7 +40,19 @@ export const FeedbackDialog: FunctionComponent = ({ "idle" | "submitting" | "submitted" >("idle"); - const { isOpen, close } = useDialog({ onClose, initialValue: true }); + const closed = useRef(false); + const notifyClose = useCallback(() => { + if (closed.current) return; + closed.current = true; + onClose?.(); + }, [onClose]); + + // Replacing a form unmounts it. Notify on that path too, but not again after an explicit close. + useLayoutEffect(() => notifyClose, [notifyClose]); + const { isOpen, close } = useDialog({ + onClose: notifyClose, + initialValue: true, + }); const autoClose = useTimer({ enabled: position.type === "DIALOG", diff --git a/packages/browser-sdk/src/feedback/ui/index.ts b/packages/browser-sdk/src/feedback/ui/index.ts index cb6307f44..2076f67e6 100644 --- a/packages/browser-sdk/src/feedback/ui/index.ts +++ b/packages/browser-sdk/src/feedback/ui/index.ts @@ -4,6 +4,7 @@ import { feedbackContainerId, propagatedEvents } from "../../ui/constants"; import { Position } from "../../ui/types"; import { FeedbackDialog } from "./FeedbackDialog"; import { OpenFeedbackFormOptions } from "./types"; +import { withFeedbackSubmissionCallback } from "./withFeedbackSubmissionCallback"; export const DEFAULT_POSITION: Position = { type: "DIALOG", @@ -79,7 +80,11 @@ export function openFeedbackForm(options: OpenFeedbackFormOptions): void { openInstances++; render( - h(FeedbackDialog, { ...options, position, key: openInstances.toString() }), + h(FeedbackDialog, { + ...withFeedbackSubmissionCallback(options), + position, + key: openInstances.toString(), + }), shadowRoot, ); } diff --git a/packages/browser-sdk/src/feedback/ui/types.ts b/packages/browser-sdk/src/feedback/ui/types.ts index 79d7d5912..64589f641 100644 --- a/packages/browser-sdk/src/feedback/ui/types.ts +++ b/packages/browser-sdk/src/feedback/ui/types.ts @@ -41,6 +41,20 @@ export interface OpenFeedbackFormOptions { openWithCommentVisible?: boolean; onSubmit: (data: FeedbackSubmission) => Promise | void; + + /** + * Handle a copy of submitted feedback, for example to send it to analytics + * or your own CRM. Called once per form: after submitting the form, or when + * it closes with a saved score without a form submission. Replacing the + * form with another feedback form also counts as closing it. + * + * On close, includes the latest saved score and an empty comment. Comment + * text that was not submitted is never included. If a save is in flight, + * the callback waits for it to finish. Closing without feedback does not + * call the callback. + */ + onAfterSubmit?: (data: FeedbackSubmission) => void; + onScoreSubmit?: ( data: FeedbackScoreSubmission, ) => Promise; diff --git a/packages/browser-sdk/src/feedback/ui/withFeedbackSubmissionCallback.ts b/packages/browser-sdk/src/feedback/ui/withFeedbackSubmissionCallback.ts new file mode 100644 index 000000000..9b120532b --- /dev/null +++ b/packages/browser-sdk/src/feedback/ui/withFeedbackSubmissionCallback.ts @@ -0,0 +1,71 @@ +import { FeedbackSubmission, OpenFeedbackFormOptions } from "./types"; + +/** Share the once-per-form callback lifecycle between manual and automated feedback. */ +export function withFeedbackSubmissionCallback( + options: OpenFeedbackFormOptions, +): OpenFeedbackFormOptions { + const { onAfterSubmit, onClose, onScoreSubmit, onSubmit } = options; + if (!onAfterSubmit) return options; + + let closed = false; + let notified = false; + let pendingSubmissions = 0; + let scoreSubmission = 0; + let savedScoreSubmission = 0; + let feedback: FeedbackSubmission | undefined; + + const notify = () => { + if (notified || !feedback) return; + // Set this before calling user code, including callbacks that throw or close the form. + notified = true; + onAfterSubmit(feedback); + }; + + const notifyOnClose = () => { + if (closed && pendingSubmissions === 0) notify(); + }; + + return { + ...options, + onScoreSubmit: onScoreSubmit + ? async (data) => { + const submission = ++scoreSubmission; + pendingSubmissions++; + try { + const result = await onScoreSubmit(data); + if (result.feedbackId && submission > savedScoreSubmission) { + savedScoreSubmission = submission; + feedback = { + ...data, + feedbackId: result.feedbackId, + comment: "", + }; + } + return result; + } finally { + pendingSubmissions--; + notifyOnClose(); + } + } + : undefined, + onSubmit: async (data) => { + pendingSubmissions++; + try { + await onSubmit(data); + feedback = data; + notify(); + } finally { + pendingSubmissions--; + notifyOnClose(); + } + }, + onClose: () => { + closed = true; + try { + notifyOnClose(); + } finally { + onClose?.(); + } + }, + }; +} diff --git a/packages/browser-sdk/src/ui/Dialog.tsx b/packages/browser-sdk/src/ui/Dialog.tsx index 784c55c4f..28da75904 100644 --- a/packages/browser-sdk/src/ui/Dialog.tsx +++ b/packages/browser-sdk/src/ui/Dialog.tsx @@ -239,6 +239,10 @@ export const Dialog: FunctionComponent = ({ ref={setDiagRef} class={classes} style={anchor ? floatingStyles : unanchoredPosition} + onCancel={(event) => { + event.preventDefault(); + dismiss(); + }} > {children && {children}} diff --git a/packages/browser-sdk/test/e2e/feedback-widget.browser.spec.ts b/packages/browser-sdk/test/e2e/feedback-widget.browser.spec.ts index 6183b7037..e24581d95 100644 --- a/packages/browser-sdk/test/e2e/feedback-widget.browser.spec.ts +++ b/packages/browser-sdk/test/e2e/feedback-widget.browser.spec.ts @@ -4,8 +4,12 @@ import { expect, Locator, Page, test } from "@playwright/test"; import { InitOptions } from "../../src/client"; import { DEFAULT_TRANSLATIONS } from "../../src/feedback/ui/config/defaultTranslations"; -import { FeedbackTranslations } from "../../src/feedback/ui/types"; +import { + FeedbackSubmission, + FeedbackTranslations, +} from "../../src/feedback/ui/types"; import { feedbackContainerId, propagatedEvents } from "../../src/ui/constants"; +import { deferred } from "../deferred"; const KEY = randomUUID(); const API_HOST = `https://front.reflag.com`; @@ -16,6 +20,8 @@ const WINDOW_HEIGHT = 720; declare global { interface Window { eventsFired: Record; + feedbackSubmissions: FeedbackSubmission[]; + requestTestFeedback: () => void; } } @@ -26,6 +32,7 @@ function pick(options: T[]): T { async function getOpenedWidgetContainer( page: Page, initOptions: Omit = {}, + source: "widget" | "prompt" = "widget", ) { await page.goto("http://localhost:8001/test/e2e/empty.html"); @@ -48,12 +55,31 @@ async function getOpenedWidgetContainer( await page.evaluate(` ;(async () => { const { ReflagClient } = await import("/dist/reflag-browser-sdk.mjs"); - const reflag = new ReflagClient({publishableKey: "${KEY}", user: {id: "foo"}, company: {id: "bar"}, ...${JSON.stringify(initOptions ?? {})}}); - await reflag.initialize(); - await reflag.requestFeedback({ - flagKey: "flag1", - title: "baz", + const options = ${JSON.stringify(initOptions)}; + window.feedbackSubmissions = []; + const onAfterSubmit = (data) => window.feedbackSubmissions.push(data); + let onMessage; + const reflag = new ReflagClient({ + publishableKey: "${KEY}", user: {id: "foo"}, company: {id: "bar"}, ...options, + feedback: { + ...options.feedback, + autoFeedbackHandler: (_prompt, handlers) => handlers.openFeedbackForm({ onAfterSubmit }), + }, + eventSourceFactory: () => ({ + addEventListener: (type, handler) => { if (type === "message") onMessage = handler; }, + close: () => {}, + }), }); + await reflag.initialize(); + window.requestTestFeedback = () => reflag.requestFeedback({ flagKey: "flag1", title: "baz", onAfterSubmit }); + if ("${source}" === "prompt") { + onMessage({ data: JSON.stringify({ + question: "baz", promptId: "prompt1", featureId: "feature1", + showAfter: Date.now() - 1000, showBefore: Date.now() + 60000, + }) }); + } else { + window.requestTestFeedback(); + } })() `); @@ -496,6 +522,99 @@ test("Closes the dialog shortly after submitting", async ({ page }) => { await expect.poll(() => isFeedbackDialogOpen(page)).toBe(false); }); +for (const source of ["widget", "prompt"] as const) { + test.describe(`${source} feedback submission callback`, () => { + const saved = { question: "baz", score: 4, feedbackId: "123", comment: "" }; + const submissions = (page: Page) => + page.evaluate(() => window.feedbackSubmissions); + + test.beforeEach(async ({ page }) => { + await page.route(`${API_HOST}/feedback`, (route) => + route.fulfill({ json: { feedbackId: "123" } }), + ); + }); + + test("reports the latest saved score on close, without a draft comment", async ({ + page, + }) => { + const container = await getOpenedWidgetContainer( + page, + { feedback: { ui: { position: { type: "MODAL" } } } }, + source, + ); + await setScore(container, 2); + await setScore(container, 4); + await setComment(container, "Unsubmitted draft"); + await expect( + container.locator(".form-expanded-content button"), + ).toBeEnabled(); + expect(await submissions(page)).toEqual([]); + await page.keyboard.press("Escape"); + await expect.poll(() => submissions(page)).toEqual([saved]); + }); + + test("reports Send once, not again on auto-close", async ({ page }) => { + const container = await getOpenedWidgetContainer(page, {}, source); + await setScore(container, 4); + await setComment(container, "Useful!"); + await submitForm(container); + const expected = [{ ...saved, comment: "Useful!" }]; + await expect.poll(() => submissions(page)).toEqual(expected); + await expect.poll(() => isFeedbackDialogOpen(page)).toBe(false); + expect(await submissions(page)).toEqual(expected); + }); + + test("closing during Send waits for the submitted comment", async ({ + page, + }) => { + const pending = deferred(); + let saving = false; + await page.route(`${API_HOST}/feedback`, async (route) => { + if (route.request().postDataJSON().comment) { + saving = true; + await pending.promise; + } + await route.fulfill({ json: { feedbackId: "123" } }); + }); + const container = await getOpenedWidgetContainer(page, {}, source); + await setScore(container, 4); + await setComment(container, "Useful!"); + await submitForm(container); + await expect.poll(() => saving).toBe(true); + await container.locator(".dialog .close").click(); + expect(await submissions(page)).toEqual([]); + pending.resolve(); + await expect + .poll(() => submissions(page)) + .toEqual([{ ...saved, comment: "Useful!" }]); + }); + + test("replacement waits for a pending score save and does not notify for the empty new form", async ({ + page, + }) => { + const pending = deferred(); + let saving = false; + await page.route(`${API_HOST}/feedback`, async (route) => { + saving = true; + await pending.promise; + await route.fulfill({ json: { feedbackId: "123" } }); + }); + const container = await getOpenedWidgetContainer(page, {}, source); + await setScore(container, 4); + await expect.poll(() => saving).toBe(true); + await page.evaluate(() => window.requestTestFeedback()); + await expect( + container.locator("#reflag-feedback-score-4"), + ).not.toBeChecked(); + expect(await submissions(page)).toEqual([]); + pending.resolve(); + await expect.poll(() => submissions(page)).toEqual([saved]); + await container.locator(".dialog .close").click(); + expect(await submissions(page)).toEqual([saved]); + }); + }); +} + test("Blocks event propagation to the containing document", async ({ page, }) => {