sentry icon indicating copy to clipboard operation
sentry copied to clipboard

feat(insights): keep transaction summary pages within the domain view

Open DominikB2014 opened this issue 1 year ago • 1 comments

If I navigate to performance -> frontend in the sidebar, then i click a transaction, I end up in the transaction summary page.

This PR updates the header for that page, and ensure all of the following tabs remain within the frontend domain view. (i.e when clicking on these tabs, the resulting url will still be under /performance/frontend/summary, currently it goes back to /performance/summary)

image

We have to also do the sub links as well in another PR, but doing this one step at a time.

DominikB2014 avatar Oct 11 '24 19:10 DominikB2014

Codecov Report

Attention: Patch coverage is 31.74603% with 43 lines in your changes missing coverage. Please review.

:white_check_mark: All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...pp/views/performance/transactionSummary/header.tsx 13.15% 33 Missing :warning:
.../transactionSummary/transactionAnomalies/utils.tsx 0.00% 2 Missing :warning:
...app/views/performance/transactionSummary/utils.tsx 50.00% 2 Missing :warning:
static/app/views/performance/utils/index.tsx 50.00% 1 Missing and 1 partial :warning:
static/app/views/performance/table.tsx 66.66% 1 Missing :warning:
...ransactionSummary/aggregateSpanWaterfall/utils.tsx 0.00% 1 Missing :warning:
...ce/transactionSummary/transactionProfiles/utils.ts 0.00% 1 Missing :warning:
...nce/transactionSummary/transactionReplays/utils.ts 0.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #79056      +/-   ##
==========================================
- Coverage   78.38%   78.33%   -0.06%     
==========================================
  Files        7140     7133       -7     
  Lines      315440   314125    -1315     
  Branches    51550    51306     -244     
==========================================
- Hits       247261   246069    -1192     
+ Misses      61711    61630      -81     
+ Partials     6468     6426      -42     

codecov[bot] avatar Oct 11 '24 19:10 codecov[bot]

Makes sense to me! There's a lot of drilling on DomainView props. I wonder if there's an easy way to reduce that, but turning those url functions into hooks probably involves a lot of cascading updates.

@edwardgou-sentry Yeah, I definitely am not a fan of the drilling down props everywhere. I wanted to use hooks everywhere, I wasn't thinking about the react update side, it was more to do with links that are sometimes generated within a class based component, or within their own helper function, making using hooks impossible or more work (like a wrapper component). There's also many more links to update within the app too.

The performance pages use prop drilling pretty extensively, so i figured it would be best to continue doing it to help get this feature out, make sure everything works as intended, and then go for a refactor to use hooks where it makes sense.

DominikB2014 avatar Oct 23 '24 19:10 DominikB2014