Skip to content

minor: try to fix firewall rule test flake again #2993

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
david-crespo merged 1 commit into from
Dec 18, 2025
Merged

minor: try to fix firewall rule test flake again #2993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions app/forms/firewall-rules-common.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,12 @@ const ProtocolFilters = ({ control }: { control: Control<FirewallRuleValues> })
placeholder=""
validate={icmpCodeValidation}
transform={normalizeDashes}
onKeyDown={(e) => {
if (e.key === KEYS.enter) {
e.preventDefault() // prevent full form submission
submitProtocol(e)
}
}}
Copy link
Collaborator Author

@david-crespo david-crespo Dec 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This wasn't causing the flake because we weren't pressing enter in this field in the test, but it was wrong. You could submit the whole form by pressing enter in this field.

/>
)}
</>
Expand Down
16 changes: 9 additions & 7 deletions app/ui/lib/Combobox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,14 +213,16 @@ export const Combobox = ({
onInputChange?.(value)
}}
onKeyDown={(e) => {
// If the caller is using onEnter to override enter behavior, preventDefault
// in order to prevent the containing form from being submitted too. We don't
// need to do this when the combobox is open because that enter keypress is
// already handled internally (selects the highlighted item). So we only do
// this when the combobox is closed.
if (e.key === 'Enter' && onEnter && !open) {
// When the combobox is open, Enter is handled internally by
// Headless UI (selects the highlighted item). When closed,
// we need to prevent the default behavior to avoid submitting
// the containing form. We also considered always preventing
// default on Enter regardless of open status, but it appears
// to break the combobox handling. Headless UI likely checks
// e.defaultPrevented before processing item selection.
if (e.key === 'Enter' && !open) {
e.preventDefault()
onEnter(e)
onEnter?.(e)
}
}}
placeholder={placeholder}
Expand Down
4 changes: 0 additions & 4 deletions test/e2e/firewall-rules.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,10 +456,6 @@ test('can update firewall rule', async ({ page }) => {
await selectOption(page, 'Protocol filters', 'ICMP')
await page.getByRole('combobox', { name: 'ICMP Type' }).fill('3')
await page.getByRole('combobox', { name: 'ICMP Type' }).press('Enter')

// give FF time to process the above enter before moving to the next field
await sleep(300)

await page.getByRole('textbox', { name: 'ICMP Code' }).fill('0')
await page.getByRole('button', { name: 'Add protocol' }).click()

Expand Down
Loading