Update protocol filters

[charliepark]

There's a little bit of design input still coming on this for the mini table and badges in the Firewall Rules tab, but I'm getting to the point where I'm fiddling, so it's time to open up the draft PR.

The crux of this is a change logged in issue #2846, where the API now uses a more sophisticated object for protocol filters, rather than the earlier strings of TCP, UDP, and ICMP. Also, users can add multiple ICMP filters. We still continue to use TCP and UDP, but ICMP protocol filters now have additional information, including an optional ICMP type and optional code.

For more on ICMP types and codes, this list of IANA code fields might be useful.

There are two main changes to the UI that a user will see with this PR.

First, the Firewall Rules tab has a new treatment for the protocol filters:

Note how the code can have either an individual number or a range.

Second, the Protocol Filters section of the Firewall Rules form has a new step-through form process, and a new mini-table:

I am open to any thoughts / tweaks / etc.

Closes #2846

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
console	✅ Ready (Inspect)	Visit Preview	Jul 17, 2025 11:04pm

[david-crespo]

The popping in and popping out listboxes and inputs feel kinda bad, though it's probably livable. This would be a great place for inline table cell editing — I think it would feel a lot more intuitive to add an ICMP row and then be able to fill in the type and the code inline. I wonder if we could reduce the cascading effect by putting the type and code inputs side by side instead of stacking them. We don't do that anywhere I can think of, though, and the combobox items probably need more room than half width.

[charliepark]

I checked in with @FelixMcFelix on the UI using the latest Vercel deployment and he had no notes, apart from liking the ICMP type names in the dropdown.

[charliepark]

The popping in and popping out listboxes and inputs feel kinda bad, though it's probably livable. This would be a great place for inline table cell editing — I think it would feel a lot more intuitive to add an ICMP row and then be able to fill in the type and the code inline. I wonder if we could reduce the cascading effect by putting the type and code inputs side by side instead of stacking them. We don't do that anywhere I can think of, though, and the combobox items probably need more room than half width.

I wonder if it might make sense to have just the protocol selector and then, once the protocol was selected (but not yet submitted) to show a summary/details type arrow to reveal the type and code options. That would give people the ability to drill down as necessary, but leave the default as "all traffic".

[benjaminleonard]

Inline inputs in the mini table might work if the content is short enough – is it just a single digit / range of single digits? I'm not against horizontally stacking the inputs also – we're not already but we don't have many inputs with single-digit items.

FWIW, we need the input mini table at some point to resolve the firewall rule form changes and webhooks anyway.

[david-crespo]

I think they really are single digits and digit ranges, though the handy type descriptions in the combobox list are a lot longer. We don't have to put those in the input itself necessarily.

[charliepark]

Tried out the side-by-side, but this feels busy to me

Open to thoughts.

[benjaminleonard]

Yeah, also realising its not worth to go too deep on this because it's all changing to mini-tables anyway. I'd say lets just squeeze in a version that works and then move onto the mini-table input.

[david-crespo]

This all looks pretty good to me. I found one UX issue that we might decide we can just live with: when you type in an invalid value in the ICMP type field and then blur rather than hitting enter or clicking the Custom entry, you end up with text in the field, but the actual value is still considered blank, so when you submit, it just goes through as if you hadn't typed, rather than showing a validation error.

I say maybe we can tolerate this because this is how the arbitrary values combobox works. We could have it clear input on blur if there's text typed that hasn't been committed to the underlying state (we wouldn't only do this when the underlying state is unset — I think we'd want it to work the same way if you've already selected a value and then type in some nonsense), but that would only help the UX when the user blurs separately from the submit click. Most of the time I would expect the submit click itself to be the event that blurs, in which case there's no point clearing on blur because it's already going to disappear when the submit goes through and the protocol filter subform resets.

I guess the best UX I can picture is that if you submit the subform when there is stuff typed in but not committed to the field state, we would treat the blur/submit as an attempt to set whatever is typed in as the state so that we can trigger a validation error and prevent the surprising thing happening. Switching to the inline-edit minitable avoids in this probably when adding the filter, but we have the same problem upon submission of the full form: you have something invalid typed in the field but it wasn't set as form state so it wasn't validated, and when you submit the form treats it like nothing was there. This is arguably even worse than the current situation because at least now, you might notice the entry in the filters table is missing a type.

2025-07-14-icmp-type-blank.mp4

Edit

I just noticed that in this particular case, you cannot submit the subform without blurring the field because the combobox popover covers it. So maybe reset on blur would help here. I would want to think through whether there are situations where that's bad.

Edit 2

If instead of clearing on blur we attempt to set the typed-in value as the actual value (analogous to pressing enter) we would immediately get the desired validation error, and this is perfectly fine for both deliberate-click-elsewhere blur and submit-click blur. So maybe that's the best thing.

[benjaminleonard]

Edit 2

If instead of clearing on blur we attempt to set the typed-in value as the actual value (analogous to pressing enter) we would immediately get the desired validation error, and this is perfectly fine for both deliberate-click-elsewhere blur and submit-click blur. So maybe that's the best thing.

Going to make an issue for more general listbox/combobox improvements but this seems like the best solution right now.

[charliepark]

Going to make an issue for more general listbox/combobox improvements but this seems like the best solution right now.

There's an improvement in #2829 that we can pull in that might improve some of that UX

[david-crespo]

I noticed that the other arbitrary values combobox on this page (the only one in the whole app, it turns out) does not have this problem because it uses onInputChange to update the form value on every character typed. That is better than onBlur. In e3a7710 I did the same for the ICMP type field and added a test (and confirmed it fails when I remove onInputChange, and confirmed a test also fails when I remove onInputChange from the other combobox). I think that is fine for now, though while investigating I decided our Combobox implementation is way too complicated internally, especially if we're doing this onInputChange thing every time we use allowArbitraryValues.

I'll have to take another quick look over, but most likely this PR is good to go.

[david-crespo]

Welp, that test failure is not a flake.

Edit: I was confused. I got two emails indicating two different failures, but they were both on 6145e42 (the main merge), while the latest commit seems to be fine.