-
Notifications
You must be signed in to change notification settings - Fork 5.7k
feat: add properties & activity to event filtering #3516
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
base: dev
Are you sure you want to change the base?
Conversation
|
@vicke4 is attempting to deploy a commit to the umami-software Team on Vercel. A member of the Team first needs to authorize it. |
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.
Greptile Summary
This PR enhances the event filtering functionality across the Umami analytics platform by making the filtering behavior more consistent across all views. The changes introduce a unified filtering experience where selecting an event label affects not just the chart (as before) but also the activity and properties tabs.
Key changes:
EventsPage.tsx: Added label state management and propagation to child componentsEventsDataTable.tsx: Implemented filtering logic to show only events matching the selected labelEventProperties.tsx: Added label-based filtering to maintain consistency with other views
The implementation uses a straightforward filtering approach by passing a label prop through the component hierarchy and filtering data based on eventName matches. This creates a more intuitive and cohesive user experience.
Confidence score: 5/5
- This PR is extremely safe to merge as it adds filtering functionality without modifying existing behavior
- The changes are well-structured, maintain type safety, and include defensive programming (default values for destructuring)
- Key files to review: All three modified files work together cohesively, but special attention should be paid to EventsPage.tsx as it manages the shared state
3 files reviewed, 2 comments
Edit PR Review Bot Settings | Greptile
| label?: string; | ||
| teamId?: string; | ||
| children?: ReactNode; |
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.
style: teamId and children props are unused but still in interface
| import styles from './EventProperties.module.css'; | ||
|
|
||
| export function EventProperties({ websiteId }: { websiteId: string }) { | ||
| export function EventProperties({ websiteId, label }: { websiteId: string; label: string }) { |
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.
style: Update interface or add JSDoc to document that label is optional since code handles null case
| export function EventProperties({ websiteId, label }: { websiteId: string; label: string }) { | |
| export function EventProperties({ websiteId, label }: { websiteId: string; label?: string }) { |
When filtering custom events by clicking on them, only the chart gets filtered. This PR filters the activity & properties tabs based on the custom event selected.