Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/swift-colts-sing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@cube-dev/ui-kit": patch
---

Fix position of ComboBox and Select popovers.
18 changes: 9 additions & 9 deletions src/components/fields/ComboBox/ComboBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
Styles,
tasty,
} from '../../../tasty';
import { chainRaf } from '../../../utils/raf';
import { generateRandomId } from '../../../utils/random';
import {
mergeProps,
Expand Down Expand Up @@ -867,17 +868,16 @@ function ComboBoxOverlay({
popoverRef as any,
);

// Update position when overlay opens
// Update position when overlay opens or content changes
useLayoutEffect(() => {
if (isOpen) {
// Use double RAF to ensure layout is complete before positioning
requestAnimationFrame(() => {
requestAnimationFrame(() => {
updatePosition?.();
});
});
if (isOpen && updatePosition) {
// Use triple RAF to ensure layout is complete before positioning
// This gives enough time for the DisplayTransition and content to render
return chainRaf(() => {
updatePosition();
}, 3);
}
}, [isOpen, updatePosition]);
}, [isOpen]);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Overlay Positioning Hook Dependency Issue

The useLayoutEffect for overlay positioning in Select and ComboBox components omits updatePosition from its dependency array. This violates React's rules of hooks, causing a stale closure. As a result, the popover's position might not update correctly if its configuration changes.

Additional Locations (1)

Fix in Cursor Fix in Web


// Extract primary placement direction for consistent styling
const placementDirection = placement?.split(' ')[0] || direction;
Expand Down
16 changes: 8 additions & 8 deletions src/components/fields/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
Styles,
tasty,
} from '../../../tasty/index';
import { chainRaf } from '../../../utils/raf';
import { generateRandomId } from '../../../utils/random';
import {
forwardRefWithGenerics,
Expand Down Expand Up @@ -525,15 +526,14 @@ export function ListBoxPopup({

// Update position when overlay opens
useLayoutEffect(() => {
if (state.isOpen) {
// Use double RAF to ensure layout is complete before positioning
requestAnimationFrame(() => {
requestAnimationFrame(() => {
updatePosition?.();
});
});
if (state.isOpen && updatePosition) {
// Use triple RAF to ensure layout is complete before positioning
// This gives enough time for the DisplayTransition and content to render
return chainRaf(() => {
updatePosition();
}, 3);
}
}, [state.isOpen, updatePosition]);
}, [state.isOpen]);

// Get props for the listbox
let { listBoxProps } = useListBox(
Expand Down
42 changes: 42 additions & 0 deletions src/utils/raf.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
* Chains N requestAnimationFrame calls and returns a cancel function.
* Useful for ensuring layout is complete before performing operations.
*
* @param callback - The function to call after N frames
* @param count - Number of RAF cycles to wait (default: 1)
* @returns A function to cancel the pending RAF call
*
* @example
* const cancel = chainRaf(() => {
* updatePosition();
* }, 3);
*
* // Later, if needed:
* cancel();
*/
export function chainRaf(callback: () => void, count: number = 1): () => void {
let rafId: number | null = null;
let cancelled = false;

const scheduleNext = (remaining: number) => {
if (cancelled) return;

if (remaining <= 0) {
callback();
return;
}

rafId = requestAnimationFrame(() => {
scheduleNext(remaining - 1);
});
};

scheduleNext(count);

return () => {
cancelled = true;
if (rafId !== null) {
cancelAnimationFrame(rafId);
}
};
}
Loading