feat: Add advanced filters panel to incident list#126
feat: Add advanced filters panel to incident list#126spalmurray wants to merge 20 commits intomainfrom
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
spalmurray
force-pushed
the
spalmurray/advanced-filters-ui
branch
from
ccb3a5c to
8e2b626
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
spalmurray
force-pushed
the
spalmurray/advanced-filters-ui
branch
from
055d7fe to
ec862c8
Compare
| to: '/', | ||
| search: {}, | ||
| replace: true, | ||
| }); | ||
| }} |
There was a problem hiding this comment.
Bug: Clicking "Clear all filters" incorrectly removes the status URL parameter, resetting the user's selected status tab to the default.
Severity: MEDIUM
Suggested Fix
Update the onClick handler in FilterTrigger.tsx to preserve existing URL search parameters when clearing filters. Instead of search: {}, use a functional update like search: s => ({ ...s, ...clearedFilters }) to only remove the relevant filter keys, preserving others like status.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: frontend/src/routes/components/filters/FilterTrigger.tsx#L19-L23
Potential issue: The "Clear all filters" button in `FilterTrigger.tsx` clears all URL
search parameters by calling `navigate` with `search: {}`. When a user is on a
non-default status tab, such as "In Review", the `status` is stored in the URL. If the
user applies advanced filters and then clicks "Clear all filters", this action will
remove not only the advanced filter parameters but also the `status` parameter, causing
the view to unexpectedly reset to the default "Active" status tab.
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
…unselectable text
spalmurray
force-pushed
the
spalmurray/advanced-filters-ui
branch
from
ec862c8 to
19df7d2
Compare
| )} | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
PillFilter and TagFilter have heavily duplicated JSX
Low Severity
PillFilter and TagFilter share ~120 lines of nearly identical JSX (header, editing mode, display mode, dropdown, backdrop). They differ only in how individual options are rendered (Pill component vs plain text). The shared state logic is already in useFilterEditor, but the rendering duplication means any bug fix or feature change needs to be applied in both places, risking inconsistency.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 19df7d2. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a4b6bc0. Configure here.
| )} | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
PillFilter and TagFilter have near-identical template code
Low Severity
PillFilter and TagFilter share ~120 lines of nearly identical JSX — the header, edit mode input, display mode tags, dropdown list, and backdrop overlay are all copy-pasted. The only meaningful differences are item rendering (Pill component vs plain text) and data source (static options prop vs useQuery). A single generic component accepting a renderItem callback and an options array would eliminate this duplication and ensure future fixes (e.g., accessibility, styling) apply consistently.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a4b6bc0. Configure here.
nsdeschenes
left a comment
nsdeschenes
left a comment
There was a problem hiding this comment.
Okay so noticed two things happening here:
-
When i leave the filters they disappear then reappear, which is a little confusing I'm thinking there's some state that is happening in between you committing them to the URL, and in those cases you should just optimistically set them as the pills.
-
There seems to be some issues with some of the fetched filters not actually filling in the tag values see near the end of the recording
Screen.Recording.2026-04-10.at.07.40.33.mov
There was a problem hiding this comment.
Did you try implementing these search params using the built-in setup from TS Router? That brings along validation and TypeSafety 👀
Docs: https://tanstack.com/router/latest/docs/guide/search-params
| ) : ( | ||
| <button | ||
| type="button" | ||
| className="text-size-sm text-content-disabled cursor-pointer select-none italic" |
There was a problem hiding this comment.
nit: I would add a hover effect here alongside the cursor just so that it "feels" more clickable
| </div> | ||
|
|
||
| <div className={cn('relative', isEditing && 'z-50')}> | ||
| {isEditing ? ( |
There was a problem hiding this comment.
nit: Instead of these long nested ternaries, i would take the hit on slightly duplicated code and move to more desecrate if statements that return the component for that condition.
It makes things a little easier to read and understand, rather than having to ruffle through looking for the ternaries.
| )} | ||
| </div> | ||
|
|
||
| {isEditing && ( |
There was a problem hiding this comment.
nit: Use always use the ternary never the double ampersand 🙀 (just to break the habit =P)
https://kentcdodds.com/blog/use-ternaries-rather-than-and-and-in-jsx
There was a problem hiding this comment.
Same thing in here with the large ternaries, tho if you struggle to break them up, i would move them into their own local component just to keep things a bit tidier imo
2 participants
Adds an expandable advanced filters panel to the incident list page with filters for severity, service tier, impact type, affected service, affected region, root cause, captain, reporter, and created date range. Filters use a draft/commit pattern where selections are stored locally during editing and flushed to URL params on close. The grid layout is responsive (1 col → 2 col → 3 col). Captain and reporter filters support server-side search with infinite scroll.