Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(replay): show screen name in url bar for mobile replays #70294

Merged
merged 2 commits into from
May 6, 2024
Merged

feat(replay): show screen name in url bar for mobile replays #70294

merged 2 commits into from
May 6, 2024

Conversation

michellewzhang
Copy link
Member

trim.mov

@michellewzhang michellewzhang requested a review from a team as a code owner May 3, 2024 23:12
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 3, 2024
michellewzhang
michellewzhang commented May 3, 2024
View reviewed changes
static/app/utils/replays/types.tsx
to: string;
from?: string;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

marked as optional because the SDK currently isn't sending a field for from but we can reuse this crumb type

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That could be a good comment on top of the prop

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll pose it a a question in the channel

@michellewzhang michellewzhang requested review from a team and c298lee May 3, 2024 23:47
@codecov Codecov
Copy link

codecov bot commented May 3, 2024

Codecov Report

Attention: Patch coverage is 8.69565% with 21 lines in your changes are missing coverage. Please review.

Project coverage is 79.90%. Comparing base (3e90887) to head (3be17e7).

❗ Current head 3be17e7 differs from pull request most recent head 859dd6b. Consider uploading reports for the commit 859dd6b to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #70294      +/-   ##
==========================================
+ Coverage   77.11%   79.90%   +2.79%     
==========================================
  Files        6503     6505       +2     
  Lines      289883   289905      +22     
  Branches    49918    49921       +3     
==========================================
+ Hits       223540   231653    +8113     
+ Misses      65720    57842    -7878     
+ Partials      623      410     -213
Files Coverage Δ
...ponents/events/eventReplay/replayPreviewPlayer.tsx 97.14% <100.00%> (ø)
static/app/components/replays/replayView.tsx 0.00% <ø> (ø)
static/app/utils/replays/types.tsx 70.73% <ø> (ø)
static/app/utils/replays/replayReader.tsx 83.06% <25.00%> (-1.30%) ⬇️
static/app/utils/replays/getCurrentScreenName.tsx 0.00% <0.00%> (ø)
...tic/app/components/replays/replayCurrentScreen.tsx 0.00% <0.00%> (ø)

... and 487 files with indirect coverage changes

@michellewzhang michellewzhang merged commit 69a9dbc into master May 6, 2024
41 of 42 checks passed
@michellewzhang michellewzhang deleted the mz/screen-name branch May 6, 2024 14:34
@michellewzhang michellewzhang mentioned this pull request May 6, 2024
Merged
michellewzhang added a commit that referenced this pull request May 6, 2024
@michellewzhang
chore(replay): add comment in types.tsx (#70349)
38d7536 
followup from
#70294 (comment)

We decided to not send `from` for now:
<img width="376" alt="SCR-20240506-kqau"
src="https://github.com/getsentry/sentry/assets/56095982/d3eeb2a4-9c3d-4d4c-ad24-71514def5531">
@sentry-io Sentry.io
Copy link

sentry-io bot commented May 8, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ TypeError: Cannot read properties of undefined (reading 'to') /replays/:replaySlug/ View Issue
  • ‼️ TypeError: T.data is undefined /replays/:replaySlug/ View Issue

Did you find this useful? React with a 👍 or 👎

@github-actions github-actions bot locked and limited conversation to collaborators May 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bruno-garcia bruno-garcia bruno-garcia left review comments

@c298lee c298lee c298lee approved these changes

Assignees
No one assigned
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Replay Details - UI] URL should show screen name
3 participants
@michellewzhang @bruno-garcia @c298lee