Skip to content

Commit 96c57cc

Browse files
roncohenroncohen
committed
Adjust dialog open styles
1 parent 1bdd5d5 commit 96c57cc

4 files changed

Lines changed: 161 additions & 32 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
.dialog {
22
position: fixed;
33
width: 210px;
4+
overflow: visible;
45
padding: 16px 22px 10px;
56
font-size: var(--reflag-feedback-dialog-font-size, 1rem);
67
font-family: var(

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
margin: auto;
4343
margin-top: 4rem;
4444

45-
&[open] {
45+
&.open {
4646
animation: /* easeOutQuint */
4747
scale 100ms cubic-bezier(0.22, 1, 0.36, 1),
4848
fade 100ms cubic-bezier(0.22, 1, 0.36, 1);
@@ -59,7 +59,7 @@
5959
position: absolute;
6060
margin: 0;
6161

62-
&[open] {
62+
&.open {
6363
animation: /* easeOutQuint */
6464
scale 100ms cubic-bezier(0.22, 1, 0.36, 1),
6565
fade 100ms cubic-bezier(0.22, 1, 0.36, 1);
@@ -92,7 +92,7 @@
9292

9393
/* Unanchored */
9494

95-
.dialog[open].unanchored {
95+
.dialog.open.unanchored {
9696
&.unanchored-bottom-left,
9797
&.unanchored-bottom-right {
9898
animation: /* easeOutQuint */

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

Lines changed: 73 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ export const Dialog: FunctionComponent<OpenDialogOptions> = ({
8181
showArrow = true,
8282
}) => {
8383
const arrowRef = useRef<HTMLDivElement>(null);
84-
const dialogRef = useRef<HTMLDialogElement>(null);
84+
const dialogRef = useRef<HTMLElement>(null);
8585

8686
const anchor = position.type === "POPOVER" ? position.anchor : null;
8787
const placement =
@@ -173,23 +173,56 @@ export const Dialog: FunctionComponent<OpenDialogOptions> = ({
173173
// eslint-disable-next-line react-hooks/exhaustive-deps -- anchor only exists in popover
174174
}, [position.type, close, (position as any).anchor, dismiss, containerId]);
175175

176-
function setDiagRef(node: HTMLDialogElement | null) {
176+
function setDiagRef(node: HTMLElement | null) {
177177
refs.setFloating(node);
178178
dialogRef.current = node;
179179
}
180180

181181
useEffect(() => {
182182
if (!dialogRef.current) return;
183-
if (isOpen && !dialogRef.current.hasAttribute("open")) {
184-
dialogRef.current[position.type === "MODAL" ? "showModal" : "show"]();
183+
184+
const isPopoverOpen = () => {
185+
try {
186+
return dialogRef.current?.matches(":popover-open") ?? false;
187+
} catch {
188+
return false;
189+
}
190+
};
191+
192+
const isModalOpen =
193+
dialogRef.current instanceof HTMLDialogElement &&
194+
dialogRef.current.hasAttribute("open");
195+
196+
if (
197+
isOpen &&
198+
((position.type === "MODAL" && !isModalOpen) ||
199+
(position.type !== "MODAL" && !isPopoverOpen()))
200+
) {
201+
if (
202+
position.type === "MODAL" &&
203+
dialogRef.current instanceof HTMLDialogElement
204+
) {
205+
dialogRef.current.showModal();
206+
} else {
207+
dialogRef.current.showPopover();
208+
}
185209
}
186-
if (!isOpen && dialogRef.current.hasAttribute("open")) {
187-
dialogRef.current.close();
210+
if (!isOpen) {
211+
if (
212+
position.type === "MODAL" &&
213+
dialogRef.current instanceof HTMLDialogElement &&
214+
dialogRef.current.hasAttribute("open")
215+
) {
216+
dialogRef.current.close();
217+
} else if (position.type !== "MODAL" && isPopoverOpen()) {
218+
dialogRef.current.hidePopover();
219+
}
188220
}
189221
}, [dialogRef, isOpen, position.type]);
190222

191223
const classes = [
192224
"dialog",
225+
isOpen ? "open" : "",
193226
position.type === "MODAL"
194227
? "modal"
195228
: position.type === "POPOVER"
@@ -201,21 +234,40 @@ export const Dialog: FunctionComponent<OpenDialogOptions> = ({
201234
return (
202235
<>
203236
<style dangerouslySetInnerHTML={{ __html: styles }} />
204-
<dialog
205-
ref={setDiagRef}
206-
class={classes}
207-
style={anchor ? floatingStyles : unanchoredPosition}
208-
>
209-
{children && <Fragment>{children}</Fragment>}
210-
211-
{anchor && showArrow && (
212-
<DialogArrow
213-
arrowData={middlewareData?.arrow}
214-
arrowRef={arrowRef}
215-
placement={actualPlacement}
216-
/>
217-
)}
218-
</dialog>
237+
{position.type === "MODAL" ? (
238+
<dialog
239+
ref={setDiagRef}
240+
class={classes}
241+
style={anchor ? floatingStyles : unanchoredPosition}
242+
>
243+
{children && <Fragment>{children}</Fragment>}
244+
245+
{anchor && showArrow && (
246+
<DialogArrow
247+
arrowData={middlewareData?.arrow}
248+
arrowRef={arrowRef}
249+
placement={actualPlacement}
250+
/>
251+
)}
252+
</dialog>
253+
) : (
254+
<div
255+
ref={setDiagRef}
256+
class={classes}
257+
style={anchor ? floatingStyles : unanchoredPosition}
258+
popover="manual"
259+
>
260+
{children && <Fragment>{children}</Fragment>}
261+
262+
{anchor && showArrow && (
263+
<DialogArrow
264+
arrowData={middlewareData?.arrow}
265+
arrowRef={arrowRef}
266+
placement={actualPlacement}
267+
/>
268+
)}
269+
</div>
270+
)}
219271
</>
220272
);
221273
};

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

Lines changed: 84 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,42 @@ async function submitForm(container: Locator) {
115115
await container.locator(".form-expanded-content").getByRole("button").click();
116116
}
117117

118+
async function isFocusInsideFeedbackDialog(page: Page) {
119+
return await page.evaluate((containerElementId) => {
120+
const container = document.querySelector(`#${containerElementId}`);
121+
122+
if (!container || !container.shadowRoot) return false;
123+
124+
let activeElement: Element | null = document.activeElement;
125+
while (activeElement instanceof HTMLElement && activeElement.shadowRoot) {
126+
const shadowActiveElement = activeElement.shadowRoot.activeElement;
127+
if (!shadowActiveElement) break;
128+
activeElement = shadowActiveElement;
129+
}
130+
131+
return activeElement ? container.shadowRoot.contains(activeElement) : false;
132+
}, feedbackContainerId);
133+
}
134+
135+
async function isFeedbackDialogOpen(page: Page) {
136+
return await page.evaluate((containerElementId) => {
137+
const container = document.querySelector(`#${containerElementId}`);
138+
const dialog = container?.shadowRoot?.querySelector(".dialog");
139+
if (!dialog) return false;
140+
141+
const modalOpen = dialog.hasAttribute("open");
142+
let popoverOpen = false;
143+
144+
try {
145+
popoverOpen = dialog.matches(":popover-open");
146+
} catch {
147+
popoverOpen = false;
148+
}
149+
150+
return modalOpen || popoverOpen;
151+
}, feedbackContainerId);
152+
}
153+
118154
test.beforeEach(async ({ page, browserName }) => {
119155
// Log any calls to front.reflag.com which aren't mocked by subsequent
120156
// `page.route` calls. With page.route, the last matching mock takes
@@ -142,7 +178,7 @@ test("Opens a feedback widget", async ({ page }) => {
142178
const container = await getOpenedWidgetContainer(page);
143179

144180
await expect(container).toBeAttached();
145-
await expect(container.locator("dialog")).toHaveAttribute("open", "");
181+
await expect.poll(() => isFeedbackDialogOpen(page)).toBe(true);
146182
});
147183

148184
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 ({
152188

153189
await page.getByTestId("give-feedback-button").click();
154190
await expect(container).toBeAttached();
155-
await expect(container.locator("dialog")).toHaveAttribute("open", "");
191+
await expect.poll(() => isFeedbackDialogOpen(page)).toBe(true);
156192

157-
await container.locator("dialog .close").click();
158-
await expect(container.locator("dialog")).not.toHaveAttribute("open", "");
193+
await container.locator(".dialog .close").click();
194+
await expect.poll(() => isFeedbackDialogOpen(page)).toBe(false);
159195

160196
await page.getByTestId("give-feedback-button").click();
161197
await expect(container).toBeAttached();
162-
await expect(container.locator("dialog")).toHaveAttribute("open", "");
198+
await expect.poll(() => isFeedbackDialogOpen(page)).toBe(true);
199+
});
200+
201+
test("Does not steal focus in DIALOG mode", async ({ page }) => {
202+
await getGiveFeedbackPageContainer(page, {
203+
feedback: {
204+
ui: {
205+
position: {
206+
type: "DIALOG",
207+
placement: "bottom-right",
208+
},
209+
},
210+
},
211+
});
212+
213+
await page.getByTestId("give-feedback-button").focus();
214+
await expect(page.getByTestId("give-feedback-button")).toBeFocused();
215+
216+
await page.getByTestId("give-feedback-button").click();
217+
await expect.poll(() => isFeedbackDialogOpen(page)).toBe(true);
218+
await expect.poll(() => isFocusInsideFeedbackDialog(page)).toBe(false);
219+
});
220+
221+
test("Steals focus in MODAL mode", async ({ page }) => {
222+
await getGiveFeedbackPageContainer(page, {
223+
feedback: {
224+
ui: {
225+
position: {
226+
type: "MODAL",
227+
},
228+
},
229+
},
230+
});
231+
232+
await page.getByTestId("give-feedback-button").focus();
233+
await expect(page.getByTestId("give-feedback-button")).toBeFocused();
234+
235+
await page.getByTestId("give-feedback-button").click();
236+
await expect.poll(() => isFeedbackDialogOpen(page)).toBe(true);
237+
await expect(page.getByTestId("give-feedback-button")).not.toBeFocused();
238+
await expect.poll(() => isFocusInsideFeedbackDialog(page)).toBe(true);
163239
});
164240

165241
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 ({
169245

170246
await expect(container).toBeAttached();
171247

172-
const bbox = await container.locator("dialog").boundingBox();
248+
const bbox = await container.locator(".dialog").boundingBox();
173249
expect(bbox?.x).toEqual(WINDOW_WIDTH - bbox!.width - 16);
174250
expect(bbox?.y).toBeGreaterThan(WINDOW_HEIGHT - bbox!.height - 30); // Account for browser differences
175251
expect(bbox?.y).toBeLessThan(WINDOW_HEIGHT - bbox!.height);
@@ -191,7 +267,7 @@ test("Opens a feedback widget in the correct position when overridden", async ({
191267

192268
await expect(container).toBeAttached();
193269

194-
const bbox = await container.locator("dialog").boundingBox();
270+
const bbox = await container.locator(".dialog").boundingBox();
195271
expect(bbox?.x).toEqual(16);
196272
expect(bbox?.y).toBeGreaterThan(0); // Account for browser differences
197273
expect(bbox?.y).toBeLessThanOrEqual(16);
@@ -411,7 +487,7 @@ test("Closes the dialog shortly after submitting", async ({ page }) => {
411487
await setComment(container, "Test comment!");
412488
await submitForm(container);
413489

414-
await expect(container.locator("dialog")).not.toHaveAttribute("open", "");
490+
await expect.poll(() => isFeedbackDialogOpen(page)).toBe(false);
415491
});
416492

417493
test("Blocks event propagation to the containing document", async ({

0 commit comments

Comments
 (0)