Skip to content

Commit 54c42b6

Browse files
fix: popover flakiness
1 parent 6ae0665 commit 54c42b6

File tree

4 files changed

+22
-18
lines changed

4 files changed

+22
-18
lines changed

apps/website/src/routes/docs/headless/popover/examples/programmatic.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ export default component$(() => {
77
<button
88
class="popover-invoker"
99
preventdefault:click
10-
onKeyDown$={(e) => {
10+
onKeyDown$={async (e) => {
1111
if (e.key === 'o') {
12-
togglePopover();
12+
await togglePopover();
1313
}
1414
}}
1515
>

packages/kit-headless/src/components/popover/popover-impl.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ export const PopoverImpl = component$<PopoverImplProps>((props) => {
102102
hasTopLayerAncestorSig.value = true;
103103
}
104104

105-
document.dispatchEvent(new CustomEvent('showpopover'));
105+
document.dispatchEvent(new CustomEvent('showpopoverpoly'));
106106

107107
cleanup(() => popoverRef.value);
108108
}

packages/kit-headless/src/components/popover/popover-trigger.tsx

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export function usePopover(popovertarget: string) {
2020

2121
const didInteractSig = useSignal<boolean>(false);
2222
const popoverSig = useSignal<HTMLElement | null>(null);
23+
const initialClickSig = useSignal<boolean>(false);
2324

2425
const loadPolyfill$ = $(async () => {
2526
await import('@oddbird/popover-polyfill');
@@ -42,30 +43,24 @@ export function usePopover(popovertarget: string) {
4243
didInteractSig.value = true;
4344
});
4445

45-
useTask$(({ track }) => {
46+
useTask$(async ({ track }) => {
4647
track(() => didInteractSig.value);
4748

4849
if (!isBrowser) return;
4950

5051
// get popover
5152
if (!popoverSig.value) {
5253
popoverSig.value = document.getElementById(popovertarget);
53-
}
54-
55-
// so it only runs once on click for supported browsers
56-
if (isSupportedSig.value) {
57-
if (!popoverSig.value) return;
5854

59-
if (popoverSig.value && popoverSig.value.hasAttribute('popover')) {
60-
/* opens manual on any event */
61-
popoverSig.value.showPopover();
55+
if (!initialClickSig.value) {
56+
popoverSig.value?.showPopover();
6257
}
6358
}
6459
});
6560

6661
// event is created after teleported properly
6762
useOnDocument(
68-
'showpopover',
63+
'showpopoverpoly',
6964
$(() => {
7065
if (!didInteractSig.value) return;
7166

@@ -74,7 +69,10 @@ export function usePopover(popovertarget: string) {
7469
// calls code in here twice for some reason, we think it's because of the client re-render, but it still works
7570

7671
// so it only runs once on first click
77-
if (!popoverSig.value.classList.contains(':popover-open')) {
72+
if (
73+
!popoverSig.value.classList.contains(':popover-open') &&
74+
!isSupportedSig.value
75+
) {
7876
popoverSig.value.showPopover();
7977
}
8078
}),
@@ -101,12 +99,12 @@ export function usePopover(popovertarget: string) {
10199
popoverSig.value?.hidePopover();
102100
});
103101

104-
return { showPopover, togglePopover, hidePopover, initPopover$ };
102+
return { showPopover, togglePopover, hidePopover, initPopover$, initialClickSig };
105103
}
106104

107105
export const PopoverTrigger = component$<PopoverTriggerProps>(
108106
({ popovertarget, disableClickInitPopover = false, ...rest }: PopoverTriggerProps) => {
109-
const { initPopover$ } = usePopover(popovertarget);
107+
const { initPopover$, initialClickSig } = usePopover(popovertarget);
110108

111109
return (
112110
<button
@@ -116,6 +114,7 @@ export const PopoverTrigger = component$<PopoverTriggerProps>(
116114
rest.onClick$,
117115
!disableClickInitPopover
118116
? $(async () => {
117+
initialClickSig.value = true;
119118
await initPopover$();
120119
})
121120
: undefined,

packages/kit-headless/src/components/popover/popover.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,9 +236,14 @@ test.describe('Keyboard Behavior', () => {
236236

237237
await expect(d.getPopover()).toBeHidden();
238238

239+
const programmaticTrigger = page.getByRole('button', {
240+
name: "Focus me and press the 'o'",
241+
});
242+
239243
// Using `page` here because driver is scoped to the popover
240-
await page.getByRole('button', { name: "Focus me and press the 'o'" }).focus();
241-
await page.keyboard.type('o');
244+
await expect(programmaticTrigger).toBeVisible();
245+
await programmaticTrigger.focus();
246+
await programmaticTrigger.press('o');
242247

243248
await expect(d.getPopover()).toBeVisible();
244249
});

0 commit comments

Comments
 (0)