fix(explorer): clear quick filter when goto jumps between resource types - #541
Conversation
A committed quick filter stayed in m.filterText across a g-prefix goto (e.g. gd from pods to deployments), silently hiding every row in the destination list. gotoResourceType now mirrors the descend path's filter bookkeeping: save the old level's filter for back-nav restore, then start the destination clean (filter, preset, search highlight). Closes TASK-839
WalkthroughNavigation between resource types, security views, and the port-forwards pseudo-list now clears live quick-filter and search state while preserving the source filter for back-navigation. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/app/whichkey_test.go`:
- Around line 307-333: Extend the gotoResourceType test around the existing
filter-state assertions to set filterBroadMode before the jump, then verify it
is cleared on the destination model and preserved in filterMemory[oldKey] for
back-navigation restoration. Keep the existing checks for the other filter
fields unchanged.
In `@internal/app/whichkey.go`:
- Around line 103-115: Update the manual filter reset in gotoResourceType to
also set m.filterBroadMode to false, ensuring broad-mode state does not carry
into the destination resource type alongside the other cleared filter fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8943678e-2c58-44ba-b903-9b28a07ab9cd
📒 Files selected for processing (2)
internal/app/whichkey.gointernal/app/whichkey_test.go
| // Mirror the descend path's filter bookkeeping: remember this level's | ||
| // committed filter for back-nav restore, then start the destination list | ||
| // clean so the old type's quick filter doesn't silently hide every row | ||
| // (TASK-839). Deliberately no restoreLevelFilter here — like a descend, a | ||
| // goto is a fresh start; only back-nav (navigateParent) recalls a saved | ||
| // filter. | ||
| m.saveLevelFilter() | ||
| m.filterText = "" | ||
| m.filterInput.Clear() | ||
| m.filterActive = false | ||
| m.activeFilterPreset = nil | ||
| m.unfilteredMiddleItems = nil | ||
| m.searchInput.Clear() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear filterBroadMode to fully reset the quick filter state.
Since gotoResourceType manually clears the filtering state to provide a fresh start (deliberately skipping restoreLevelFilter), it should also explicitly reset m.filterBroadMode = false. Otherwise, if the broad filter mode (e.g., the Tab toggle) was active on the old resource type, it will leak to the destination level and unexpectedly apply when the user types a new filter.
🐛 Proposed fix
m.saveLevelFilter()
m.filterText = ""
m.filterInput.Clear()
m.filterActive = false
+ m.filterBroadMode = false
m.activeFilterPreset = nil
m.unfilteredMiddleItems = nil
m.searchInput.Clear()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Mirror the descend path's filter bookkeeping: remember this level's | |
| // committed filter for back-nav restore, then start the destination list | |
| // clean so the old type's quick filter doesn't silently hide every row | |
| // (TASK-839). Deliberately no restoreLevelFilter here — like a descend, a | |
| // goto is a fresh start; only back-nav (navigateParent) recalls a saved | |
| // filter. | |
| m.saveLevelFilter() | |
| m.filterText = "" | |
| m.filterInput.Clear() | |
| m.filterActive = false | |
| m.activeFilterPreset = nil | |
| m.unfilteredMiddleItems = nil | |
| m.searchInput.Clear() | |
| // Mirror the descend path's filter bookkeeping: remember this level's | |
| // committed filter for back-nav restore, then start the destination list | |
| // clean so the old type's quick filter doesn't silently hide every row | |
| // (TASK-839). Deliberately no restoreLevelFilter here — like a descend, a | |
| // goto is a fresh start; only back-nav (navigateParent) recalls a saved | |
| // filter. | |
| m.saveLevelFilter() | |
| m.filterText = "" | |
| m.filterInput.Clear() | |
| m.filterActive = false | |
| m.filterBroadMode = false | |
| m.activeFilterPreset = nil | |
| m.unfilteredMiddleItems = nil | |
| m.searchInput.Clear() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/app/whichkey.go` around lines 103 - 115, Update the manual filter
reset in gotoResourceType to also set m.filterBroadMode to false, ensuring
broad-mode state does not carry into the destination resource type alongside the
other cleared filter fields.
UAT feedback on the goto fix: the filter must start clean on any path that lands on a different resource list, not just g-prefix chords. Extract the save-then-clear bookkeeping into resetFilterForTypeSwitch and use it on every type-switch path: regular descend (navigateChild), goto chords, the security-finding teleport, the per-resource findings view, and the port-forwards list. The last three previously leaked the origin list's filter. Broad-match mode now also clears (and is saved for back-nav restore), per CodeRabbit review. Bookmark jumps and session restore intentionally keep applying their own saved filters.
|
@coderabbitai Both review comments are addressed in e1d74ac: the filter reset (now the shared |
|
Tip For best results, initiate chat on the files or code changes.
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
1 participant
Summary
resetFilterForTypeSwitch: save the origin's filter for back-nav restore, then clear filter text/input, broad mode, preset state, and search highlight. Covered paths: regular descend, g-prefix goto chords (gd,gv, ...), the security-finding teleport, the per-resource findings view, and the port-forwards list. The last three previously leaked.navigateParent) recalls a saved filter. Bookmark jumps and session restore keep applying their own saved filters by design.Test plan
TestGotoResourceType_ClearsQuickFilter(red before fix) +TestGotoResourceType_DoesNotRestoreDestinationFilterfilter_typeswitch_test.go: regular-nav pin + red-first leak tests for the finding teleport, findings view, and port-forwards pathsgo test -race ./...clean,golangci-lint run ./...0 issues,go vet ./...cleanTracked as Backlog TASK-839; also implements TASK-840 (
jumpToFindingResourceleak).