diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index b597bd786..91fd8ec0c 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -494,6 +494,7 @@ const ProtocolFilters = ({ control }: { control: Control }) description="Leave blank to match any type" placeholder="" allowArbitraryValues + onEnter={submitProtocol} items={icmpTypeItems[selectedProtocolType]} validate={(value) => { const result = parseIcmpType(value) diff --git a/app/ui/lib/Combobox.browser.spec.tsx b/app/ui/lib/Combobox.browser.spec.tsx index ed2aa8190..570125fcd 100644 --- a/app/ui/lib/Combobox.browser.spec.tsx +++ b/app/ui/lib/Combobox.browser.spec.tsx @@ -41,20 +41,26 @@ function ComboboxHarness({ comboboxItems = items, initialValue = '', allowArbitraryValues = false, + onEnter, }: { comboboxItems?: ComboboxItem[] initialValue?: string allowArbitraryValues?: boolean + onEnter?: () => void }) { const [selectedItemValue, setSelectedItemValue] = useState(initialValue) const [parentEscapes, setParentEscapes] = useState(0) + const [submits, setSubmits] = useState(0) return ( // The parent key handler models the modal form's Escape listener. // oxlint-disable-next-line jsx-a11y/no-noninteractive-element-interactions
event.preventDefault()} + onSubmit={(event) => { + event.preventDefault() + setSubmits((count) => count + 1) + }} onKeyDown={(event) => { if (event.key === 'Escape') setParentEscapes((count) => count + 1) }} @@ -70,9 +76,12 @@ function ComboboxHarness({ onChange={setSelectedItemValue} onInputChange={allowArbitraryValues ? setSelectedItemValue : undefined} allowArbitraryValues={allowArbitraryValues} + onEnter={onEnter} /> + Selected: {selectedItemValue || '(empty)'} Parent escapes: {parentEscapes} + Submits: {submits}
) } @@ -197,6 +206,36 @@ test('Enter commits the highlighted option instead of the arbitrary query', asyn await expect.element(screen.getByText('Selected: mock-vpc')).toBeVisible() }) +test('Enter selects while open, then submits the form once closed', async () => { + const screen = await render() + const combobox = screen.getByRole('combobox', { name: 'Disk name' }) + + await commands.clickComboboxButton('Disk name') + await expect.element(combobox).toHaveAttribute('aria-expanded', 'true') + await commands.pressComboboxKey('Disk name', 'Home') + await commands.pressComboboxKey('Disk name', 'Enter') + await expect.element(screen.getByText('Selected: disk-3')).toBeVisible() + await expect.element(screen.getByText('Submits: 0')).toBeVisible() + + await commands.pressComboboxKey('Disk name', 'Enter') + await expect.element(screen.getByText('Submits: 1')).toBeVisible() +}) + +test('onEnter replaces form submission when closed', async () => { + const onEnter = vi.fn() + const screen = await render() + + await commands.clickComboboxButton('Disk name') + await commands.pressComboboxKey('Disk name', 'Home') + await commands.pressComboboxKey('Disk name', 'Enter') + await expect.element(screen.getByText('Selected: disk-3')).toBeVisible() + expect(onEnter).not.toHaveBeenCalled() + + await commands.pressComboboxKey('Disk name', 'Enter') + await expect.poll(() => onEnter.mock.calls.length).toBe(1) + await expect.element(screen.getByText('Submits: 0')).toBeVisible() +}) + test('Escape closes the options before propagating to the parent', async () => { const screen = await render() const combobox = screen.getByRole('combobox', { name: 'Disk name' }) diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx index d03368a31..b93465718 100644 --- a/app/ui/lib/Combobox.tsx +++ b/app/ui/lib/Combobox.tsx @@ -73,7 +73,11 @@ export type ComboboxBaseProps = { * For example, if a form value should be set when the user types, use `onInputChange`. */ onInputChange?: (value: string) => void - /** Fires whenever the Enter key is pressed while Combobox input has focus */ + /** + * Fires whenever the Enter key is pressed while Combobox input has focus and + * the dropdown is closed. Additionally, it prevents default, so when set it + * will disable form submission on Enter press. + */ onEnter?: (event: React.KeyboardEvent) => void /** * Optional function to transform the value entered into the input as the user types. @@ -278,11 +282,12 @@ export const Combobox = ({ }} onKeyDown={(e) => { // When the dropdown is open, Enter should select the - // highlighted option (HUI handles this). When closed, - // Enter should submit the subform via onEnter. - if (e.key === 'Enter' && !isOpenRef.current) { + // highlighted option (HUI handles this). When closed, Enter + // should either fire the onEnter handler or, implicitly, + // submit the form it's within. + if (e.key === 'Enter' && !isOpenRef.current && onEnter) { e.preventDefault() - onEnter?.(e) + onEnter(e) } }} placeholder={placeholder} diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts index 5d7243763..28ed53655 100644 --- a/test/e2e/firewall-rules.e2e.ts +++ b/test/e2e/firewall-rules.e2e.ts @@ -639,3 +639,22 @@ test('can add ICMPv4 and ICMPv6 protocol filters', async ({ page }) => { await page.getByRole('option', { name: 'ICMPv4', exact: true }).click() await expect(page.getByRole('combobox', { name: 'ICMPv4 type' })).toHaveValue('') }) + +test('enter in the ICMP type field adds a protocol filter', async ({ page }) => { + await page.goto('/projects/mock-project/vpcs/default/firewall-rules-new') + + await page.getByRole('button', { name: 'Protocol filters' }).click() + await page.getByRole('option', { name: 'ICMPv4', exact: true }).click() + + const v4Type = page.getByRole('combobox', { name: 'ICMPv4 type' }) + await fillAndSelectComboboxOption(v4Type, page, '8', '8 - Echo Request') + await v4Type.press('Enter') + + await expectRowVisible(page.getByRole('table', { name: 'Protocol filters' }), { + Protocol: 'ICMPv4', + Type: '8', + }) + + // the form itself should not have been submitted + await expect(page.getByRole('dialog', { name: 'Add firewall rule' })).toBeVisible() +})