fix(browser-sdk): call onAfterSubmit once when scored feedback closes

.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.

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);

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,

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<FeedbackDialogProps> = ({
     "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",

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,
   );
 }

packages/browser-sdk/src/feedback/ui/types.ts

@@ -41,6 +41,20 @@ export interface OpenFeedbackFormOptions {
   openWithCommentVisible?: boolean;
 
   onSubmit: (data: FeedbackSubmission) => Promise<void> | 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<OnScoreSubmitResult>;

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?.();
+      }
+    },
+  };
+}

packages/browser-sdk/src/ui/Dialog.tsx

@@ -239,6 +239,10 @@ export const Dialog: FunctionComponent<OpenDialogOptions> = ({
           ref={setDiagRef}
           class={classes}
           style={anchor ? floatingStyles : unanchoredPosition}
+          onCancel={(event) => {
+            event.preventDefault();
+            dismiss();
+          }}
         >
           {children && <Fragment>{children}</Fragment>}
 

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<string, boolean>;
+    feedbackSubmissions: FeedbackSubmission[];
+    requestTestFeedback: () => void;
   }
 }
 
@@ -26,6 +32,7 @@ function pick<T>(options: T[]): T {
 async function getOpenedWidgetContainer(
   page: Page,
   initOptions: Omit<InitOptions, "publishableKey"> = {},
+  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<void>();
+      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<void>();
+      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,
 }) => {