Skip to content

Fix firewall rule test flake in Firefox #3059

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 4 commits into from
Feb 8, 2026
Merged

Fix firewall rule test flake in Firefox #3059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b1870d6
try to fix test flake in ff
david-crespo Feb 7, 2026
d311380
format lol
david-crespo Feb 7, 2026
0e68a50
fix for safari
david-crespo Feb 7, 2026
43f9b3e
format, add formatting post tool use hook
david-crespo Feb 7, 2026
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
16 changes: 16 additions & 0 deletions .claude/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"hooks": {
"PostToolUse": [
{
"matcher": "Edit|Write|NotebookEdit",
"hooks": [
{
"type": "command",
"command": "jq -r '.tool_input.file_path' | xargs ./node_modules/.bin/prettier --write --ignore-unknown",
"timeout": 10
}
]
}
]
}
}
255 changes: 135 additions & 120 deletions app/ui/lib/Combobox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
} from '@headlessui/react'
import cn from 'classnames'
import { matchSorter } from 'match-sorter'
import { useEffect, useId, useState, type ReactNode, type Ref } from 'react'
import { useEffect, useId, useRef, useState, type ReactNode, type Ref } from 'react'

import { SelectArrows6Icon } from '@oxide/design-system/icons/react'

Expand Down Expand Up @@ -145,146 +145,161 @@ export const Combobox = ({
}
const zIndex = usePopoverZIndex()
const id = useId()
// Tracks whether the dropdown is open so the onKeyDown handler can
// distinguish Enter-to-select (dropdown open, let HUI handle it) from
// Enter-to-submit (dropdown closed, fire onEnter). We use a ref instead
// of HUI's `open` render prop because our handler runs before HUI's
// (useRender merges user props first) and the render prop can be stale.
// The ref stays current because onClose sets it synchronously during
// HUI's own keydown handler. With the stale render prop, the handler
// could see the combobox as closed one keydown too late, firing onEnter
// instead of letting HUI select — hard to notice manually but caused
// consistent e2e flakes in Firefox.
const isOpenRef = useRef(false)
return (
<HCombobox
// necessary, as the displayed "value" is not the same as the actual selected item's *value*
value={selectedItemValue}
// fallback to '' allows clearing field to work
onChange={(val) => onChange(val || '')}
// we only want to keep the query on close when arbitrary values are allowed
onClose={allowArbitraryValues ? undefined : () => setQuery('')}
onClose={() => {
isOpenRef.current = false
if (!allowArbitraryValues) setQuery('')
}}
disabled={disabled || isLoading}
immediate
{...props}
>
{({ open }) => (
<div>
{label && (
// TODO: FieldLabel needs a real ID
<div className="mb-2">
<FieldLabel
id={`${id}-label`}
htmlFor={`${id}-input`}
optional={!required && !hideOptionalTag}
>
{label}
</FieldLabel>
{description && (
<TextInputHint id={`${id}-help-text`}>{description}</TextInputHint>
)}
</div>
)}
<div
className={cn(
`flex rounded-md border focus-within:ring-2`,
hasError
? 'focus-error border-error-secondary focus-within:ring-error-secondary hover:border-error'
: 'border-default focus-within:ring-accent-secondary hover:border-hover',
disabled
? 'text-disabled bg-disabled border-default! cursor-not-allowed'
: 'bg-default',
disabled && hasError && 'border-error-secondary!'
{({ open }) => {
// Sync open state to ref on render (handles the opening side)
if (open) isOpenRef.current = true
return (
<div>
{label && (
// TODO: FieldLabel needs a real ID
<div className="mb-2">
<FieldLabel
id={`${id}-label`}
htmlFor={`${id}-input`}
optional={!required && !hideOptionalTag}
>
{label}
</FieldLabel>
{description && (
<TextInputHint id={`${id}-help-text`}>{description}</TextInputHint>
)}
</div>
)}
// Putting the inputRef on the div makes it so the div can be focused by RHF when there's an error.
// We want to focus on the div (rather than the input) so the combobox doesn't open automatically
// and obscure the error message.
ref={inputRef}
// tabIndex=-1 is necessary to make the div focusable
tabIndex={-1}
>
<ComboboxInput
id={`${id}-input`}
// If an option has been selected, display either the selected item's label or value.
// If no option has been selected yet, or the user has started editing the input, display the query.
// We are using value here, as opposed to Headless UI's displayValue, so we can normalize
// the value entered into the input (via the onChange event).
value={
selectedItemValue
? allowArbitraryValues
? selectedItemValue
: selectedItemLabel
: query
}
onChange={(event) => {
const value = transform ? transform(event.target.value) : event.target.value
// updates the query state as the user types, in order to filter the list of items
setQuery(value)
// if the parent component wants to know about input changes, call the callback
onInputChange?.(value)
}}
onKeyDown={(e) => {
// 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)
}
}}
placeholder={placeholder}
disabled={disabled || isLoading}
<div
className={cn(
`text-sans-md text-raise placeholder:text-tertiary h-10 w-full rounded-md border-none! px-3 py-2 outline-hidden!`,
`flex rounded-md border focus-within:ring-2`,
hasError
? 'focus-error border-error-secondary focus-within:ring-error-secondary hover:border-error'
: 'border-default focus-within:ring-accent-secondary hover:border-hover',
disabled
? 'text-disabled bg-disabled border-default! cursor-not-allowed'
: 'bg-default',
hasError && 'focus-error'
disabled && hasError && 'border-error-secondary!'
)}
/>
{items.length > 0 && (
<ComboboxButton
// Putting the inputRef on the div makes it so the div can be focused by RHF when there's an error.
// We want to focus on the div (rather than the input) so the combobox doesn't open automatically
// and obscure the error message.
ref={inputRef}
// tabIndex=-1 is necessary to make the div focusable
tabIndex={-1}
>
<ComboboxInput
id={`${id}-input`}
// If an option has been selected, display either the selected item's label or value.
// If no option has been selected yet, or the user has started editing the input, display the query.
// We are using value here, as opposed to Headless UI's displayValue, so we can normalize
// the value entered into the input (via the onChange event).
value={
selectedItemValue
? allowArbitraryValues
? selectedItemValue
: selectedItemLabel
: query
}
onChange={(event) => {
const value = transform
? transform(event.target.value)
: event.target.value
// updates the query state as the user types, in order to filter the list of items
setQuery(value)
// if the parent component wants to know about input changes, call the callback
onInputChange?.(value)
}}
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) {
e.preventDefault()
onEnter?.(e)
}
}}
placeholder={placeholder}
disabled={disabled || isLoading}
className={cn(
'border-secondary my-1.5 flex items-center border-l px-3',
disabled ? 'bg-disabled cursor-not-allowed' : 'bg-default'
`text-sans-md text-raise placeholder:text-tertiary h-10 w-full rounded-md border-none! px-3 py-2 outline-hidden!`,
disabled
? 'text-disabled bg-disabled border-default! cursor-not-allowed'
: 'bg-default',
hasError && 'focus-error'
)}
aria-hidden
/>
{items.length > 0 && (
<ComboboxButton
className={cn(
'border-secondary my-1.5 flex items-center border-l px-3',
disabled ? 'bg-disabled cursor-not-allowed' : 'bg-default'
)}
aria-hidden
>
<SelectArrows6Icon title="Select" className="text-secondary w-2" />
</ComboboxButton>
)}
</div>
{(items.length > 0 || allowArbitraryValues) && (
<ComboboxOptions
anchor="bottom start"
// 13px gap is presumably because it's measured from inside the outline or something
className={`ox-menu pointer-events-auto ${zIndex} border-secondary relative w-[calc(var(--input-width)+var(--button-width))] overflow-y-auto border outline-hidden! [--anchor-gap:13px] empty:hidden`}
modal={false}
>
<SelectArrows6Icon title="Select" className="text-secondary w-2" />
</ComboboxButton>
{filteredItems.map((item) => (
<ComboboxOption
key={item.value}
value={item.value}
className="border-secondary relative border-b last:border-0"
>
{({ focus, selected }) => (
// This *could* be done with data-[focus] and data-[selected] instead, but
// it would be a lot more verbose. those can only be used with TW classes,
// not our .is-selected and .is-highlighted, so we'd have to copy the pieces
// of those rules one by one. Better to rely on the shared classes.
<div
className={cn('ox-menu-item', {
'is-selected': selected && query !== item.value,
'is-highlighted': focus,
})}
>
{item.label}
</div>
)}
</ComboboxOption>
))}
{!allowArbitraryValues && filteredItems.length === 0 && (
<ComboboxOption disabled value="no-matches" className="relative">
<div className="ox-menu-item text-disabled!">No items match</div>
</ComboboxOption>
)}
</ComboboxOptions>
)}
</div>
{(items.length > 0 || allowArbitraryValues) && (
<ComboboxOptions
anchor="bottom start"
// 13px gap is presumably because it's measured from inside the outline or something
className={`ox-menu pointer-events-auto ${zIndex} border-secondary relative w-[calc(var(--input-width)+var(--button-width))] overflow-y-auto border outline-hidden! [--anchor-gap:13px] empty:hidden`}
modal={false}
>
{filteredItems.map((item) => (
<ComboboxOption
key={item.value}
value={item.value}
className="border-secondary relative border-b last:border-0"
>
{({ focus, selected }) => (
// This *could* be done with data-[focus] and data-[selected] instead, but
// it would be a lot more verbose. those can only be used with TW classes,
// not our .is-selected and .is-highlighted, so we'd have to copy the pieces
// of those rules one by one. Better to rely on the shared classes.
<div
className={cn('ox-menu-item', {
'is-selected': selected && query !== item.value,
'is-highlighted': focus,
})}
>
{item.label}
</div>
)}
</ComboboxOption>
))}
{!allowArbitraryValues && filteredItems.length === 0 && (
<ComboboxOption disabled value="no-matches" className="relative">
<div className="ox-menu-item text-disabled!">No items match</div>
</ComboboxOption>
)}
</ComboboxOptions>
)}
</div>
)}
)
}}
</HCombobox>
)
}
Loading
Loading