ref(theme): remove backgroundSecondary
#106037
Conversation
github-actions
bot
added
the
Scope: Frontend
natemoo-re
force-pushed
the
natemoore/de-333-remove-themebackgroundsecondary
branch
2 times, most recently
from
bdf76d2 to
38e726b
Compare
|
Waiting on #106127 to unblock the test failures |
In the course of #106037, I hit a fun exception in the `TeamSelector`. Previously, the style functions accessed `state.theme` from the `react-select` callback, casting it as Sentry's global theme because it also uses Emotion. In the test environment, `react-select` provided a theme object that didn't have Sentry's `tokens` property, causing `theme.tokens.background` to fail. In this PR, we've refactored the `TeamSelector` to have receive the theme from `useTheme()` hook instead of expecting Emotion to automatically merge `react-select`'s theme with ours.
natemoo-re
force-pushed
the
natemoore/de-333-remove-themebackgroundsecondary
branch
from
38e726b to
88bb026
Compare
natemoo-re
force-pushed
the
natemoore/de-333-remove-themebackgroundsecondary
branch
from
bfa8e4f to
4365288
Compare
natemoo-re
force-pushed
the
natemoore/de-333-remove-themebackgroundsecondary
branch
from
4365288 to
adb74e2
Compare
JonasBa
left a comment
JonasBa
left a comment
There was a problem hiding this comment.
Nice, thank you for adding the :active state here 🥇
natemoo-re
force-pushed
the
natemoore/de-333-remove-themebackgroundsecondary
branch
from
1fb0d1a to
ba9090b
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| lineHeight: 12, | ||
| formatter: (value: number) => axisLabelFormatter(value, 'number', true), | ||
| textBorderColor: theme.backgroundSecondary, | ||
| textBorderColor: theme.tokens.border.secondary, |
There was a problem hiding this comment.
Wrong token used: border instead of background
Medium Severity
The textBorderColor property was changed from theme.backgroundSecondary to theme.tokens.border.secondary, but backgroundSecondary was an alias for tokens.background.secondary. The correct replacement is theme.tokens.background.secondary. Using the wrong token will result in a different color being rendered for the chart axis label text border, which could affect readability depending on the theme's border vs background color values.
| &:hover { | ||
| background-color: ${p => p.theme.backgroundSecondary}; | ||
| } | ||
There was a problem hiding this comment.
Active state feedback removed from interactive button
Low Severity
The InsightCardButton component previously had an &:active { opacity: 0.8; } rule that provided visual feedback when clicked. This active state was removed entirely rather than being replaced with the interactive token pattern (tokens.interactive.transparent.neutral.background.active) used consistently elsewhere in this PR for similar interactive elements like autofixSolutionEventItem, autofixTimelineItem, and autofixInsightCard. This inconsistency removes click feedback from a button element that has role="button" and onClick.
natemoo-re
deleted the
natemoore/de-333-remove-themebackgroundsecondary
branch
Removes
backgroundSecondaryfrom our theme, closes DE-333Recommend reviewing by commit:
049ffe6 replaced simple cases with
background.secondary587e368 replaced gradients
4117747 replaces interactive usage with correct tokens
36bf26d removes
backgroundSecondaryfrom theme