fix(codecov): Fix line coverage legend edge cases and coloring #102742

calvin-codecov · 2025-11-05T00:02:53Z

Followup to #102450 to fix edge cases of when pills have long, individual text that doesn't wrap.

Solution: drop the legend on to its own line below and shrink a little bit to prevent taking away too much attention away from the pills.

Small screen width with wrapping:

Normal case:

static/app/components/events/interfaces/crashContent/exception/content.tsx

static/app/components/events/interfaces/crashContent/exception/lineCoverageLegend.tsx

JonasBa · 2025-11-05T03:58:12Z

static/app/components/events/interfaces/crashContent/exception/lineCoverageLegend.tsx

+const CoveredLine = styled(Text)`
  padding-right: ${p => p.theme.space.md};
-  border-right: 3px solid ${p => p.theme.tokens.content.success};
+  border-right: 3px solid ${p => p.theme.green400};


We have tokens.border.success for these values and you should use those. theme.greenxxx colors and the rest were never used for borders and will be removed in the near future

JonasBa · 2025-11-05T03:58:25Z

static/app/components/events/interfaces/crashContent/exception/lineCoverageLegend.tsx

+const UncoveredLine = styled(Text)`
  padding-right: ${p => p.theme.space.md};
-  border-right: 3px solid ${p => p.theme.tokens.content.danger};
+  border-right: 3px solid ${p => p.theme.red300};


Same comment here. Use theme.tokens.border

JonasBa · 2025-11-05T03:58:51Z

static/app/components/events/interfaces/crashContent/exception/lineCoverageLegend.tsx

+const PartiallyCoveredLine = styled(Text)`
  padding-right: ${p => p.theme.space.md};
-  border-right: 3px dashed ${p => p.theme.tokens.content.warning};
+  border-right: 3px dashed ${p => p.theme.yellow300};


Same comment here. Please use theme.token.border.warning

JonasBa · 2025-11-05T04:00:42Z

static/app/components/events/interfaces/crashContent/exception/mechanism.tsx

+const Wrapper = styled('div')`
+  margin: ${p => p.theme.space.xl} 0 ${p => p.theme.space.xs} 0;
+`;


This is a harmful pattern because you are essentially building margin into a component, which can be done at the instance where the component is used. Building it into a component like this is going to make it very hard to use in different layouts.

Suggested change

const Wrapper = styled('div')`

margin: ${p => p.theme.space.xl} 0 ${p => p.theme.space.xs} 0;

`;

use <Container margin="xl 0"> if you have to at the appropriate call sites.

You make a good point. I was just reverting it back to what it was before I made any changes but we should modify this.

JonasBa · 2025-11-05T04:00:50Z

static/app/components/events/interfaces/frame/contextLineNumber.tsx

  &.covered .line-number {
    background: ${p => p.theme.green100};
-    border-right: 3px solid ${p => p.theme.tokens.content.success};
+    border-right: 3px solid ${p => p.theme.green400};


use tokens.border

Since the "background" rule right above aims for a more faded, lower numbered value version of the same color, would it be acceptable to use tokens.content.success for the border here and tokens.border.success for the background color? How should we be handling a scenario like this where we have two similar but distinct sets of colors that should work with each other.

JonasBa · 2025-11-05T04:00:59Z

static/app/components/events/interfaces/frame/contextLineNumber.tsx

  &.uncovered .line-number {
    background: ${p => p.theme.red100};
-    border-right: 3px solid ${p => p.theme.tokens.content.danger};
+    border-right: 3px solid ${p => p.theme.red300};


use tokens.border

JonasBa · 2025-11-05T04:01:05Z

static/app/components/events/interfaces/frame/contextLineNumber.tsx

  &.partial .line-number {
    background: ${p => p.theme.yellow100};
-    border-right: 3px dashed ${p => p.theme.tokens.content.warning};
+    border-right: 3px dashed ${p => p.theme.yellow300};


use tokens.border

JonasBa · 2025-11-05T04:01:52Z

static/app/components/events/interfaces/crashContent/exception/content.tsx

-const RowWrapper = styled(Flex)`
-  margin: ${p => p.theme.space.xl} 0 ${p => p.theme.space.xs} 0;
-`;


This was the better way to handle the wrapper requirement at the call site

static/app/components/events/interfaces/frame/contextLineNumber.tsx

static/app/components/events/interfaces/crashContent/exception/lineCoverageLegend.tsx

JonasBa

Nice, that is much cleaner 🥇

Followup to #102450 to fix edge cases of when pills have long, individual text that doesn't wrap. <img width="2264" height="792" alt="image" src="https://github.com/user-attachments/assets/5a2151ec-c752-4e88-9611-9b2a6698535d" /> Solution: drop the legend on to its own line below and shrink a little bit to prevent taking away too much attention away from the pills. Small screen width with wrapping: <img width="512" height="385" alt="Screenshot 2025-11-04 at 3 55 08 PM" src="https://github.com/user-attachments/assets/ca46f2af-9dac-4313-a70e-cbb6280778e1" /> Normal case: <img width="917" height="410" alt="Screenshot 2025-11-04 at 3 55 21 PM" src="https://github.com/user-attachments/assets/8dd9f47f-33a3-4508-8153-62278d1f883e" />

fix(codecov): Fix line coverage legend edge cases and coloring

7aed6d4

calvin-codecov requested a review from a team as a code owner November 5, 2025 00:02

github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Nov 5, 2025

cursor bot reviewed Nov 5, 2025

View reviewed changes

static/app/components/events/interfaces/crashContent/exception/content.tsx Outdated Show resolved Hide resolved

sentry bot reviewed Nov 5, 2025

View reviewed changes

static/app/components/events/interfaces/crashContent/exception/lineCoverageLegend.tsx Outdated Show resolved Hide resolved

scttcper approved these changes Nov 5, 2025

View reviewed changes

JonasBa reviewed Nov 5, 2025

View reviewed changes

static/app/components/events/interfaces/crashContent/exception/lineCoverageLegend.tsx Outdated Show resolved Hide resolved

JonasBa requested changes Nov 5, 2025

View reviewed changes

fix(codecov): Update component structuring review changes

63e47aa

cursor bot reviewed Nov 6, 2025

View reviewed changes

vercel bot deployed to Preview November 6, 2025 08:58 View deployment

fix(codecov): Update frame line number indication colors

43e145a

cursor bot reviewed Nov 6, 2025

View reviewed changes

static/app/components/events/interfaces/crashContent/exception/lineCoverageLegend.tsx Show resolved Hide resolved

vercel bot deployed to Preview November 6, 2025 09:17 View deployment

fix(codecov): Add back dash styling

e9a40c3

vercel bot deployed to Preview November 6, 2025 16:57 View deployment

JonasBa approved these changes Nov 7, 2025

View reviewed changes

calvin-codecov merged commit 47dd7cf into master Nov 7, 2025
48 checks passed

calvin-codecov deleted the cy/move_coverage_legend branch November 7, 2025 23:24

	const Wrapper = styled('div')`
	margin: ${p => p.theme.space.xl} 0 ${p => p.theme.space.xs} 0;
	`;

Uh oh!

fix(codecov): Fix line coverage legend edge cases and coloring #102742

fix(codecov): Fix line coverage legend edge cases and coloring #102742

Conversation

calvin-codecov commented Nov 5, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

calvin-codecov Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JonasBa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

calvin-codecov Nov 6, 2025 •

edited

Loading