Restrict feedback focus to modal

packages/browser-sdk/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@reflag/browser-sdk",
-  "version": "1.4.0",
+  "version": "1.4.1",
   "packageManager": "yarn@4.1.1",
   "license": "MIT",
   "repository": {

packages/browser-sdk/src/feedback/ui/FeedbackDialog.css

@@ -1,6 +1,7 @@
 .dialog {
   position: fixed;
   width: 210px;
+  overflow: visible;
   padding: 16px 22px 10px;
   font-size: var(--reflag-feedback-dialog-font-size, 1rem);
   font-family: var(

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

@@ -11,6 +11,14 @@ export const DEFAULT_POSITION: Position = {
   placement: "bottom-right",
 };
 
+function supportsPopoverApi() {
+  return (
+    typeof HTMLElement !== "undefined" &&
+    "showPopover" in HTMLElement.prototype &&
+    "hidePopover" in HTMLElement.prototype
+  );
+}
+
 function stopPropagation(e: Event) {
   e.stopPropagation();
 }
@@ -39,9 +47,18 @@ function attachDialogContainer() {
 let openInstances = 0;
 
 export function openFeedbackForm(options: OpenFeedbackFormOptions): void {
-  const shadowRoot = attachDialogContainer();
   const position = options.position || DEFAULT_POSITION;
 
+  if (position.type !== "MODAL" && !supportsPopoverApi()) {
+    console.warn(
+      "[Reflag]",
+      "Unable to open feedback popup. Popover API is not supported in this browser",
+    );
+    return;
+  }
+
+  const shadowRoot = attachDialogContainer();
+
   if (position.type === "POPOVER") {
     if (!position.anchor) {
       console.warn(

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

@@ -42,7 +42,7 @@
   margin: auto;
   margin-top: 4rem;
 
-  &[open] {
+  &.open {
     animation: /* easeOutQuint */
       scale 100ms cubic-bezier(0.22, 1, 0.36, 1),
       fade 100ms cubic-bezier(0.22, 1, 0.36, 1);
@@ -59,7 +59,7 @@
   position: absolute;
   margin: 0;
 
-  &[open] {
+  &.open {
     animation: /* easeOutQuint */
       scale 100ms cubic-bezier(0.22, 1, 0.36, 1),
       fade 100ms cubic-bezier(0.22, 1, 0.36, 1);
@@ -92,7 +92,7 @@
 
 /* Unanchored */
 
-.dialog[open].unanchored {
+.dialog.open.unanchored {
   &.unanchored-bottom-left,
   &.unanchored-bottom-right {
     animation: /* easeOutQuint */

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

@@ -81,7 +81,7 @@ export const Dialog: FunctionComponent<OpenDialogOptions> = ({
   showArrow = true,
 }) => {
   const arrowRef = useRef<HTMLDivElement>(null);
-  const dialogRef = useRef<HTMLDialogElement>(null);
+  const dialogRef = useRef<HTMLElement>(null);
 
   const anchor = position.type === "POPOVER" ? position.anchor : null;
   const placement =
@@ -173,23 +173,56 @@ export const Dialog: FunctionComponent<OpenDialogOptions> = ({
     // eslint-disable-next-line react-hooks/exhaustive-deps -- anchor only exists in popover
   }, [position.type, close, (position as any).anchor, dismiss, containerId]);
 
-  function setDiagRef(node: HTMLDialogElement | null) {
+  function setDiagRef(node: HTMLElement | null) {
     refs.setFloating(node);
     dialogRef.current = node;
   }
 
   useEffect(() => {
     if (!dialogRef.current) return;
-    if (isOpen && !dialogRef.current.hasAttribute("open")) {
-      dialogRef.current[position.type === "MODAL" ? "showModal" : "show"]();
+
+    const isPopoverOpen = () => {
+      try {
+        return dialogRef.current?.matches(":popover-open") ?? false;
+      } catch {
+        return false;
+      }
+    };
+
+    const isModalOpen =
+      dialogRef.current instanceof HTMLDialogElement &&
+      dialogRef.current.hasAttribute("open");
+
+    if (
+      isOpen &&
+      ((position.type === "MODAL" && !isModalOpen) ||
+        (position.type !== "MODAL" && !isPopoverOpen()))
+    ) {
+      if (
+        position.type === "MODAL" &&
+        dialogRef.current instanceof HTMLDialogElement
+      ) {
+        dialogRef.current.showModal();
+      } else {
+        dialogRef.current.showPopover();
+      }
     }
-    if (!isOpen && dialogRef.current.hasAttribute("open")) {
-      dialogRef.current.close();
+    if (!isOpen) {
+      if (
+        position.type === "MODAL" &&
+        dialogRef.current instanceof HTMLDialogElement &&
+        dialogRef.current.hasAttribute("open")
+      ) {
+        dialogRef.current.close();
+      } else if (position.type !== "MODAL" && isPopoverOpen()) {
+        dialogRef.current.hidePopover();
+      }
     }
   }, [dialogRef, isOpen, position.type]);
 
   const classes = [
     "dialog",
+    isOpen ? "open" : "",
     position.type === "MODAL"
       ? "modal"
       : position.type === "POPOVER"
@@ -201,21 +234,40 @@ export const Dialog: FunctionComponent<OpenDialogOptions> = ({
   return (
     <>
       <style dangerouslySetInnerHTML={{ __html: styles }} />
-      <dialog
-        ref={setDiagRef}
-        class={classes}
-        style={anchor ? floatingStyles : unanchoredPosition}
-      >
-        {children && <Fragment>{children}</Fragment>}
-
-        {anchor && showArrow && (
-          <DialogArrow
-            arrowData={middlewareData?.arrow}
-            arrowRef={arrowRef}
-            placement={actualPlacement}
-          />
-        )}
-      </dialog>
+      {position.type === "MODAL" ? (
+        <dialog
+          ref={setDiagRef}
+          class={classes}
+          style={anchor ? floatingStyles : unanchoredPosition}
+        >
+          {children && <Fragment>{children}</Fragment>}
+
+          {anchor && showArrow && (
+            <DialogArrow
+              arrowData={middlewareData?.arrow}
+              arrowRef={arrowRef}
+              placement={actualPlacement}
+            />
+          )}
+        </dialog>
+      ) : (
+        <div
+          ref={setDiagRef}
+          class={classes}
+          style={anchor ? floatingStyles : unanchoredPosition}
+          popover="manual"
+        >
+          {children && <Fragment>{children}</Fragment>}
+
+          {anchor && showArrow && (
+            <DialogArrow
+              arrowData={middlewareData?.arrow}
+              arrowRef={arrowRef}
+              placement={actualPlacement}
+            />
+          )}
+        </div>
+      )}
     </>
   );
 };

packages/browser-sdk/test/e2e/feedback-widget.browser.spec.ts

@@ -115,6 +115,42 @@ async function submitForm(container: Locator) {
   await container.locator(".form-expanded-content").getByRole("button").click();
 }
 
+async function isFocusInsideFeedbackDialog(page: Page) {
+  return await page.evaluate((containerElementId) => {
+    const container = document.querySelector(`#${containerElementId}`);
+
+    if (!container || !container.shadowRoot) return false;
+
+    let activeElement: Element | null = document.activeElement;
+    while (activeElement instanceof HTMLElement && activeElement.shadowRoot) {
+      const shadowActiveElement = activeElement.shadowRoot.activeElement;
+      if (!shadowActiveElement) break;
+      activeElement = shadowActiveElement;
+    }
+
+    return activeElement ? container.shadowRoot.contains(activeElement) : false;
+  }, feedbackContainerId);
+}
+
+async function isFeedbackDialogOpen(page: Page) {
+  return await page.evaluate((containerElementId) => {
+    const container = document.querySelector(`#${containerElementId}`);
+    const dialog = container?.shadowRoot?.querySelector(".dialog");
+    if (!dialog) return false;
+
+    const modalOpen = dialog.hasAttribute("open");
+    let popoverOpen = false;
+
+    try {
+      popoverOpen = dialog.matches(":popover-open");
+    } catch {
+      popoverOpen = false;
+    }
+
+    return modalOpen || popoverOpen;
+  }, feedbackContainerId);
+}
+
 test.beforeEach(async ({ page, browserName }) => {
   // Log any calls to front.reflag.com which aren't mocked by subsequent
   // `page.route` calls. With page.route, the last matching mock takes
@@ -142,7 +178,7 @@ test("Opens a feedback widget", async ({ page }) => {
   const container = await getOpenedWidgetContainer(page);
 
   await expect(container).toBeAttached();
-  await expect(container.locator("dialog")).toHaveAttribute("open", "");
+  await expect.poll(() => isFeedbackDialogOpen(page)).toBe(true);
 });
 
 test("Opens a feedback widget multiple times in same session", async ({
@@ -152,14 +188,54 @@ test("Opens a feedback widget multiple times in same session", async ({
 
   await page.getByTestId("give-feedback-button").click();
   await expect(container).toBeAttached();
-  await expect(container.locator("dialog")).toHaveAttribute("open", "");
+  await expect.poll(() => isFeedbackDialogOpen(page)).toBe(true);
 
-  await container.locator("dialog .close").click();
-  await expect(container.locator("dialog")).not.toHaveAttribute("open", "");
+  await container.locator(".dialog .close").click();
+  await expect.poll(() => isFeedbackDialogOpen(page)).toBe(false);
 
   await page.getByTestId("give-feedback-button").click();
   await expect(container).toBeAttached();
-  await expect(container.locator("dialog")).toHaveAttribute("open", "");
+  await expect.poll(() => isFeedbackDialogOpen(page)).toBe(true);
+});
+
+test("Does not steal focus in DIALOG mode", async ({ page }) => {
+  await getGiveFeedbackPageContainer(page, {
+    feedback: {
+      ui: {
+        position: {
+          type: "DIALOG",
+          placement: "bottom-right",
+        },
+      },
+    },
+  });
+
+  await page.getByTestId("give-feedback-button").focus();
+  await expect(page.getByTestId("give-feedback-button")).toBeFocused();
+
+  await page.getByTestId("give-feedback-button").click();
+  await expect.poll(() => isFeedbackDialogOpen(page)).toBe(true);
+  await expect.poll(() => isFocusInsideFeedbackDialog(page)).toBe(false);
+});
+
+test("Steals focus in MODAL mode", async ({ page }) => {
+  await getGiveFeedbackPageContainer(page, {
+    feedback: {
+      ui: {
+        position: {
+          type: "MODAL",
+        },
+      },
+    },
+  });
+
+  await page.getByTestId("give-feedback-button").focus();
+  await expect(page.getByTestId("give-feedback-button")).toBeFocused();
+
+  await page.getByTestId("give-feedback-button").click();
+  await expect.poll(() => isFeedbackDialogOpen(page)).toBe(true);
+  await expect(page.getByTestId("give-feedback-button")).not.toBeFocused();
+  await expect.poll(() => isFocusInsideFeedbackDialog(page)).toBe(true);
 });
 
 test("Opens a feedback widget in the bottom right by default", async ({
@@ -169,7 +245,7 @@ test("Opens a feedback widget in the bottom right by default", async ({
 
   await expect(container).toBeAttached();
 
-  const bbox = await container.locator("dialog").boundingBox();
+  const bbox = await container.locator(".dialog").boundingBox();
   expect(bbox?.x).toEqual(WINDOW_WIDTH - bbox!.width - 16);
   expect(bbox?.y).toBeGreaterThan(WINDOW_HEIGHT - bbox!.height - 30); // Account for browser differences
   expect(bbox?.y).toBeLessThan(WINDOW_HEIGHT - bbox!.height);
@@ -191,7 +267,7 @@ test("Opens a feedback widget in the correct position when overridden", async ({
 
   await expect(container).toBeAttached();
 
-  const bbox = await container.locator("dialog").boundingBox();
+  const bbox = await container.locator(".dialog").boundingBox();
   expect(bbox?.x).toEqual(16);
   expect(bbox?.y).toBeGreaterThan(0); // Account for browser differences
   expect(bbox?.y).toBeLessThanOrEqual(16);
@@ -411,7 +487,7 @@ test("Closes the dialog shortly after submitting", async ({ page }) => {
   await setComment(container, "Test comment!");
   await submitForm(container);
 
-  await expect(container.locator("dialog")).not.toHaveAttribute("open", "");
+  await expect.poll(() => isFeedbackDialogOpen(page)).toBe(false);
 });
 
 test("Blocks event propagation to the containing document", async ({

packages/openfeature-browser-provider/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@reflag/openfeature-browser-provider",
-  "version": "1.3.0",
+  "version": "1.3.1",
   "packageManager": "yarn@4.1.1",
   "license": "MIT",
   "repository": {
@@ -35,7 +35,7 @@
     }
   },
   "dependencies": {
-    "@reflag/browser-sdk": "1.4.0"
+    "@reflag/browser-sdk": "1.4.1"
   },
   "devDependencies": {
     "@openfeature/core": "1.5.0",

packages/react-sdk/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@reflag/react-sdk",
-  "version": "1.4.0",
+  "version": "1.4.1",
   "license": "MIT",
   "repository": {
     "type": "git",
@@ -37,7 +37,7 @@
     }
   },
   "dependencies": {
-    "@reflag/browser-sdk": "1.4.0"
+    "@reflag/browser-sdk": "1.4.1"
   },
   "peerDependencies": {
     "react": "*",

packages/vue-sdk/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@reflag/vue-sdk",
-  "version": "1.3.0",
+  "version": "1.3.1",
   "license": "MIT",
   "repository": {
     "type": "git",
@@ -35,7 +35,7 @@
     }
   },
   "dependencies": {
-    "@reflag/browser-sdk": "1.4.0"
+    "@reflag/browser-sdk": "1.4.1"
   },
   "peerDependencies": {
     "vue": "^3.0.0"