-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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(issue-details): Allow filters for first/last/recommended #82476
Conversation
Bundle ReportChanges will decrease total bundle size by 78.49kB (-0.24%) ⬇️. This is within the configured threshold ✅ Detailed changes
|
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.
looks like the preloading for prev/next events is broken with these changes on my end - mind double checking if they work for you?
i'm personally not the biggest fan of including this line - "404: Event not found. The event ID may be incorrect, or it's age exceeded the retention period." to me , it feels weird that we are explicitly saying it's a 404 and the rest of it feels like it could be bullet points in the section below |
t( | ||
'Change up your filters. Try more environments, a wider date range, or a different query' | ||
), | ||
tct('If nothing stands out, try [link:clearing your filters] all together', { |
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.
this is great
That's fair, that error message comes directly from the endpoint. Should we just omit displaying the 404 in the UI all together? The endpoint will keep the error messages for users using the API directly. |
Removed the error from being displayed and also made some context changes so that the new event count would show up in the dropdown. |
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #82476 +/- ##
=======================================
Coverage 72.48% 72.49%
=======================================
Files 7306 7306
Lines 322126 322090 -36
Branches 21011 21000 -11
=======================================
- Hits 233488 233486 -2
+ Misses 87445 87411 -34
Partials 1193 1193 |
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.
can you run this through https://svgomg.net/
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.
way less confusing 👍
This PR does a few things, but mostly addresses the inconsistencies with filtering in the streamlined UI.
Specific event missing
Query doesn't find anything
todo