fix(ourlogs): Search and display fixes for embedded views #90940
Conversation
github-actions
bot
added
the
Scope: Frontend
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 #90940 +/- ##
=======================================
Coverage 75.96% 75.96%
=======================================
Files 10296 10296
Lines 584462 584439 -23
Branches 22584 22578 -6
=======================================
- Hits 443966 443956 -10
+ Misses 140066 140053 -13
Partials 430 430 |
colin-sentry
force-pushed
the
better_frozen_handling
branch
from
6f93f04 to
c505461
Compare
colin-sentry
changed the title
| isTableFrozen | ||
| blockRowExpanding |
There was a problem hiding this comment.
are these two conditionals ever exclusive? Does a frozen table imply we're using the drawer always? Or is it only in issues where we do View More and have the limited table?
| @@ -74,7 +91,7 @@ export function LogsIssuesSection({ | |||
| // We may change this in the future if we have a trace-group or we generate trace sids for these issue types. | |||
| return null; | |||
| } | |||
| if (tableData?.data?.length === 0) { | |||
| if (!tableData || (tableData.data?.length === 0 && logsSearch.isEmpty())) { | |||
There was a problem hiding this comment.
Oh is the the bug that hides it if you are filtering down and it's an empty table? Can we make a quick test and just assert the empty state renders (mock the requests returning empty) to cover these?
There was a problem hiding this comment.
I'll do the tests for the issues page in a separate PR, there is no harness yet
| icon={<IconChevron direction="right" />} | ||
| aria-label={t('View more')} | ||
| size="md" | ||
| onClick={() => onOpenLogsDrawer()} |
There was a problem hiding this comment.
do we need both onClick?
Also I hadn't realized it but we would have never been able to expand logs for log count < 5 on the table without the general onclick, so good that we added it
There was a problem hiding this comment.
yea, you can still tab to focus it
colin-sentry
force-pushed
the
better_frozen_handling
branch
from
c505461 to
523246e
Compare
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
- Do not show cell actions in embedded tables - Add a test to such effect - Rename things to be consistent everywhere (just isFrozen) - Refactor the issues section - Do not entirely hide the section if your search returns empty results (unreported bug) - Use useState for search on embedded pages (issues, trace view) Fixes LOGS-82
Fixes LOGS-82