Skip to content
Open
Show file tree
Hide file tree
Changes from all 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: 4 additions & 1 deletion src/components/ui/Select/fragments/SelectPortal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ type SelectPortalPrimitiveProps = React.ComponentPropsWithoutRef<typeof Combobox

const SelectPortal = React.forwardRef<SelectPortalElement, SelectPortalPrimitiveProps>(({ children, container, ...props }, forwardedRef) => {
const themeContext = useContext(ThemeContext);
const portalContainer = container ?? themeContext?.portalRootRef.current;
const portalContainer = container
?? themeContext?.portalRootRef.current
?? themeContext?.containerRef.current
?? undefined;

return (
<ComboboxPrimitive.Portal ref={forwardedRef} container={portalContainer} {...props}>
Expand Down
44 changes: 44 additions & 0 deletions src/components/ui/Select/tests/Select.portal.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import React from 'react';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import Select from '../Select';
import Theme from '~/components/ui/Theme/Theme';

const mockMatchMedia = () => {
if ('matchMedia' in window && typeof window.matchMedia === 'function') return;
Object.defineProperty(window, 'matchMedia', {
writable: true,
value: jest.fn().mockImplementation(() => ({
matches: false,
addEventListener: jest.fn(),
removeEventListener: jest.fn()
}))
});
};

describe('Select portal', () => {
beforeEach(() => mockMatchMedia());

test('portals listbox into Theme portal root', async() => {
const user = userEvent.setup();

render(
<Theme>
<Select.Root>
<Select.Trigger />
<Select.Portal>
<Select.Content>
<Select.Item value="apple">Apple</Select.Item>
</Select.Content>
</Select.Portal>
</Select.Root>
</Theme>
);

const portalRoot = document.querySelector('[data-rad-ui-portal-root]') as HTMLElement;
expect(portalRoot).toBeTruthy();

await user.click(screen.getByRole('combobox'));
expect(portalRoot).toContainElement(screen.getByText('Apple'));
Comment on lines +41 to +42

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make the post-click assertion async-safe to avoid flaky tests.

After Line 41, rendering the listbox content may lag by a tick; using getByText at Line 42 can intermittently fail. Prefer awaiting findByText before asserting containment.

Proposed fix
         await user.click(screen.getByRole('combobox'));
-        expect(portalRoot).toContainElement(screen.getByText('Apple'));
+        const appleOption = await screen.findByText('Apple');
+        expect(portalRoot).toContainElement(appleOption);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await user.click(screen.getByRole('combobox'));
expect(portalRoot).toContainElement(screen.getByText('Apple'));
await user.click(screen.getByRole('combobox'));
const appleOption = await screen.findByText('Apple');
expect(portalRoot).toContainElement(appleOption);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/ui/Select/tests/Select.portal.test.tsx` around lines 41 - 42,
The assertion at Line 42 in the Select.portal.test.tsx file uses the synchronous
getByText method immediately after the click, which can cause flaky tests if the
listbox content hasn't rendered yet. Replace the getByText call with findByText
(which is async and polls for the element) and await it before checking if
portalRoot contains the element. This ensures the DOM has settled and the
'Apple' text is actually present before making the containment assertion.

});
});
Loading