diff --git a/.changeset/source-form-dropdown-clipping.md b/.changeset/source-form-dropdown-clipping.md new file mode 100644 index 0000000000..08f54d1c83 --- /dev/null +++ b/.changeset/source-form-dropdown-clipping.md @@ -0,0 +1,5 @@ +--- +'@hyperdx/app': patch +--- + +Fix the database, table, and connection dropdowns being clipped inside the source setup modal. The dropdowns now render in a portal, so the full list is visible and scrollable when configuring or editing a source. diff --git a/packages/app/src/components/ConnectionSelect.tsx b/packages/app/src/components/ConnectionSelect.tsx index 476f254394..2422377cfb 100644 --- a/packages/app/src/components/ConnectionSelect.tsx +++ b/packages/app/src/components/ConnectionSelect.tsx @@ -26,7 +26,7 @@ export function ConnectionSelectControlled({ allowDeselect={false} data={values} // disabled={isDatabasesLoading} - comboboxProps={{ withinPortal: false }} + comboboxProps={{ withinPortal: true }} searchable placeholder="Connection" leftSection={} diff --git a/packages/app/src/components/DBTableSelect.tsx b/packages/app/src/components/DBTableSelect.tsx index 0228db08f7..8d715b6280 100644 --- a/packages/app/src/components/DBTableSelect.tsx +++ b/packages/app/src/components/DBTableSelect.tsx @@ -61,7 +61,7 @@ function DBTableSelect({ data={data} disabled={isTablesLoading} value={table} - comboboxProps={{ withinPortal: false }} + comboboxProps={{ withinPortal: true }} onChange={v => setTable(v ?? undefined)} onBlur={onBlur} name={name} diff --git a/packages/app/src/components/DatabaseSelect.tsx b/packages/app/src/components/DatabaseSelect.tsx index db48b2fd76..b2270a313a 100644 --- a/packages/app/src/components/DatabaseSelect.tsx +++ b/packages/app/src/components/DatabaseSelect.tsx @@ -41,7 +41,7 @@ function DatabaseSelect({ maxDropdownHeight={280} data={data} disabled={isDatabasesLoading} - comboboxProps={{ withinPortal: false }} + comboboxProps={{ withinPortal: true }} value={database} onChange={v => setDatabase(v ?? undefined)} onBlur={onBlur} diff --git a/packages/app/src/components/__tests__/sourceFormPickerDropdowns.test.tsx b/packages/app/src/components/__tests__/sourceFormPickerDropdowns.test.tsx new file mode 100644 index 0000000000..70d2841534 --- /dev/null +++ b/packages/app/src/components/__tests__/sourceFormPickerDropdowns.test.tsx @@ -0,0 +1,150 @@ +import { useForm } from 'react-hook-form'; +import { screen, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import { useDatabasesDirect, useTablesDirect } from '@/clickhouse'; +import { useConnections } from '@/connection'; + +import { ConnectionSelectControlled } from '../ConnectionSelect'; +import { DatabaseSelectControlled } from '../DatabaseSelect'; +import { DBTableSelectControlled } from '../DBTableSelect'; + +jest.mock('@/clickhouse', () => ({ + useDatabasesDirect: jest.fn(), + useTablesDirect: jest.fn(), +})); +jest.mock('@/connection', () => ({ + useConnections: jest.fn(), +})); +// DBTableSelect renders these next to the table picker; not relevant to this test. +jest.mock('../SourceSchemaPreview', () => ({ + __esModule: true, + default: () => null, + isSourceSchemaPreviewEnabled: () => false, +})); +jest.mock('../SourceSelect', () => ({ + SourceManagementMenu: () => null, +})); + +// Mantine's Combobox calls scrollIntoView when its dropdown opens; jsdom lacks it. +window.HTMLElement.prototype.scrollIntoView = jest.fn(); + +// The hooks return large `UseQueryResult`/connection types; the components only +// read a couple of fields, so cast to a loose mock for these minimal fixtures. +const asMock = (fn: unknown) => fn as jest.Mock; + +// Use a large list so the dropdown overflows like the bug report (HDX-4445). +const OPTION_COUNT = 20; +const databaseNames = Array.from( + { length: OPTION_COUNT }, + (_, i) => `db_${String(i).padStart(2, '0')}`, +); +const tableNames = Array.from( + { length: OPTION_COUNT }, + (_, i) => `table_${String(i).padStart(2, '0')}`, +); +const connections = Array.from({ length: OPTION_COUNT }, (_, i) => ({ + id: `conn-${i}`, + name: `Connection ${String(i).padStart(2, '0')}`, +})); + +beforeEach(() => { + asMock(useDatabasesDirect).mockReturnValue({ + data: { data: databaseNames.map(name => ({ name })) }, + isLoading: false, + }); + asMock(useTablesDirect).mockReturnValue({ + data: { data: tableNames.map(name => ({ name })) }, + isLoading: false, + }); + asMock(useConnections).mockReturnValue({ data: connections }); +}); + +function DatabaseHarness() { + const { control } = useForm(); + return ( + + ); +} + +function TableHarness() { + const { control } = useForm(); + return ( + + ); +} + +function ConnectionHarness() { + const { control } = useForm(); + return ; +} + +/** + * HDX-4445: these pickers live inside the source-setup modal. With many entries + * the dropdown must (a) render every option and (b) render them in a portal, so + * the modal's overflow can't clip the list. + * + * `hidden: true` — jsdom has no layout, so the portaled dropdown computes as + * "hidden"; the default role query would skip it. + * + * Note on what jsdom can/can't prove: clipping is a *visual* effect of + * `overflow:hidden`, which does not change the DOM or an element's box, and + * jsdom has no layout — so pixel visibility ("is option 20 actually on screen") + * is not assertable here. The two checks below are what's both meaningful and + * deterministic: every option is rendered (no truncation/virtualization), and + * the options are portaled OUT of the picker's container so a modal can't clip + * them. The container check is what flips between fixed (`withinPortal: true`) + * and broken (`false`) — independent of option count. True on-screen proof + * would require a real-browser (E2E) test. + */ +async function expectAllOptionsRenderedInPortal( + container: HTMLElement, + trigger: HTMLElement, +) { + await userEvent.click(trigger); + + // (a) All options are rendered — none dropped by truncation/virtualization. + const allOptions = await screen.findAllByRole('option', { hidden: true }); + expect(allOptions).toHaveLength(OPTION_COUNT); + + // (b) None are nested inside the picker's own container: they are portaled + // out, so the modal's overflow cannot clip them. Fails on withinPortal:false. + expect( + within(container).queryAllByRole('option', { hidden: true }), + ).toHaveLength(0); +} + +describe('source form picker dropdowns render all options in a portal', () => { + it(`DatabaseSelect renders all ${OPTION_COUNT} databases in a portal`, async () => { + const { container } = renderWithMantine(); + await expectAllOptionsRenderedInPortal( + container, + screen.getByPlaceholderText('Database'), + ); + }); + + it(`DBTableSelect renders all ${OPTION_COUNT} tables in a portal`, async () => { + const { container } = renderWithMantine(); + await expectAllOptionsRenderedInPortal( + container, + screen.getByPlaceholderText('Table'), + ); + }); + + it(`ConnectionSelect renders all ${OPTION_COUNT} connections in a portal`, async () => { + const { container } = renderWithMantine(); + await expectAllOptionsRenderedInPortal( + container, + screen.getByPlaceholderText('Connection'), + ); + }); +});