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.

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

Use gray and italicized text for system frames #51758

Merged
merged 23 commits into from
Jul 18, 2023
Merged

Conversation

17hogeju
Copy link
Contributor

@17hogeju 17hogeju commented Jun 27, 2023

Closes #50691 and #50693

Distinguishes "In App" and "System" frames using gray/italicized text with no pill for "System".

Screenshot 2023-07-17 at 4 01 52 PM Screenshot 2023-07-17 at 4 01 40 PM

Non-Native stacktrace changes

Screenshot 2023-06-29 at 10 17 37 AM
Screenshot 2023-06-29 at 10 16 38 AM

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jun 27, 2023
@17hogeju 17hogeju marked this pull request as ready for review June 29, 2023 17:19
@17hogeju 17hogeju requested a review from a team as a code owner June 29, 2023 17:19
@17hogeju
Copy link
Contributor Author

One thing I noticed is that when italics are used for Native Stacktrace, the location text gets cut off a little
Screenshot 2023-06-29 at 4 24 22 PM

@malwilley
Copy link
Member

One thing I noticed is that when italics are used for Native Stacktrace, the location text gets cut off a little

@17hogeju ah good catch. We do want to fix that before merging. This is happending because the containing element has overflow: hidden (which we use to prevent overflow and apply the ellipsis for long lib names) and it a bit too small. You'll want to find a solution where the containing element takes up the full space, or maybe even some padding might help. LMK if you need help finding the right solution.

Copy link
Member

@malwilley malwilley left a comment

Choose a reason for hiding this comment

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

Need to put this behind a feature flag, requesting changes so we don't accidentally merge

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 17, 2023
@github-actions
Copy link
Contributor

🚨 Warning: This pull request contains Frontend and Backend changes!

It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently.

Have questions? Please ask in the #discuss-dev-infra channel.

This reverts commit 2689d14.
@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Merging #51758 (2689d14) into master (6affbee) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 2689d14 differs from pull request most recent head 9097ddd. Consider uploading reports for the commit 9097ddd to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #51758   +/-   ##
=======================================
  Coverage   79.45%   79.45%           
=======================================
  Files        4936     4936           
  Lines      207367   207379   +12     
  Branches    35426    35426           
=======================================
+ Hits       164756   164773   +17     
+ Misses      37572    37568    -4     
+ Partials     5039     5038    -1     
Impacted Files Coverage Δ
src/sentry/conf/server.py 91.74% <ø> (ø)
src/sentry/features/__init__.py 100.00% <ø> (ø)
...pp/components/events/eventReplay/replayPreview.tsx 95.83% <ø> (ø)
...c/app/components/events/interfaces/nativeFrame.tsx 55.88% <ø> (ø)
static/app/utils/replays/replayReader.tsx 75.00% <ø> (ø)
...app/views/performance/transactionSummary/utils.tsx 81.08% <ø> (ø)
static/app/views/replays/replayTable/tableCell.tsx 88.88% <ø> (ø)
src/sentry/api/endpoints/project_team_details.py 100.00% <100.00%> (ø)
src/sentry/apidocs/examples/project_examples.py 100.00% <100.00%> (ø)

... and 5 files with indirect coverage changes

@17hogeju 17hogeju removed the Scope: Backend Automatically applied to PRs that change backend components label Jul 17, 2023
@malwilley malwilley linked an issue Jul 17, 2023 that may be closed by this pull request
@17hogeju
Copy link
Contributor Author

Need to put this behind a feature flag, requesting changes so we don't accidentally merge

Done!

Copy link
Member

@malwilley malwilley left a comment

Choose a reason for hiding this comment

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

Great work!

@17hogeju 17hogeju merged commit 25f0725 into master Jul 18, 2023
49 checks passed
@17hogeju 17hogeju deleted the julia/remove-system-tags branch July 18, 2023 19:01
@github-actions github-actions bot locked and limited conversation to collaborators Aug 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stack trace: Remove "System" frame tags Stack trace: Use gray and italicized text for system frames
3 participants