-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: search saved filters #32610
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.
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
feat: search saved filters #32610
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR adds search functionality to session recording saved filters and improves empty state handling. The changes include a new search input field and dedicated empty state component.
- Added new
SavedFiltersEmptyStatecomponent infrontend/src/scenes/session-recordings/filters/SavedFiltersEmptyState.tsxto handle error and no-filters states - Added search input field in
frontend/src/scenes/session-recordings/filters/SavedFilters.tsxwith real-time filtering - Modified
savedSessionRecordingPlaylistsLogicto handle search state and filtering logic - Updated empty state to only show when there are no results AND no active search term
- Improved UI by always showing 'Saved filters' tab regardless of filter count
4 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
| import { savedSessionRecordingPlaylistsLogic } from '../saved-playlists/savedSessionRecordingPlaylistsLogic' | ||
|
|
||
| export function SavedFiltersEmptyState(): JSX.Element { | ||
| const playlistsLogic = savedSessionRecordingPlaylistsLogic({ tab: ReplayTabs.Playlists }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Using ReplayTabs.Playlists here seems incorrect since this is a filters view, not a playlists view
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know. The next step is separation of logics.
|
Size Change: +156 B (0%) Total Size: 3.72 MB ℹ️ View Unchanged
|
| value={savedFiltersSearch} | ||
| stopPropagation={true} | ||
| /> | ||
| <LemonTable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we pass loading to lemon table still?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, as we have states above it that do it nicer:
f (savedFiltersLoading && !savedFiltersSearch) {
return
}
if (savedFilters.results?.length === 0 && !savedFiltersSearch) {
return <SavedFiltersEmptyState />
}
| limit: PLAYLISTS_PER_PAGE, | ||
| offset: Math.max(0, (filters.page - 1) * PLAYLISTS_PER_PAGE), | ||
| order: filters.order ?? '-last_modified_at', // Sync with `sorting` selector | ||
| order: '-last_modified_at', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess filters.order is a collections thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
pauldambra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple of questions but otherwise ![]()
before
after