Skip to content

dnm - fake snapshot diff#112130

Open
NicoHinderling wants to merge 3 commits intomasterfrom
dnm-fake-snapshot-diff
Open

dnm - fake snapshot diff#112130
NicoHinderling wants to merge 3 commits intomasterfrom
dnm-fake-snapshot-diff

Conversation

@NicoHinderling
Copy link
Copy Markdown
Contributor

No description provided.

@NicoHinderling NicoHinderling requested a review from a team as a code owner April 2, 2026 18:14
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 2, 2026
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

color: theme.tokens.interactive.chonky.embossed.danger.content,
surface: theme.tokens.interactive.chonky.embossed.success.background,
background: theme.tokens.interactive.chonky.embossed.success.chonk,
color: theme.tokens.interactive.chonky.embossed.success.content,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Danger button incorrectly uses success theme tokens

High Severity

The 'danger' case in getButtonTheme now references success tokens (theme.tokens.interactive.chonky.embossed.success.*) instead of danger tokens. This causes all danger buttons to render with success styling (likely green instead of red), completely inverting their visual meaning.

Fix in Cursor Fix in Web

Comment on lines 284 to +286
return {
surface: theme.tokens.interactive.chonky.embossed.danger.background,
background: theme.tokens.interactive.chonky.embossed.danger.chonk,
color: theme.tokens.interactive.chonky.embossed.danger.content,
surface: theme.tokens.interactive.chonky.embossed.success.background,
background: theme.tokens.interactive.chonky.embossed.success.chonk,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: Buttons with priority="danger" are incorrectly styled using success theme tokens, causing them to appear green instead of red.
Severity: HIGH

Suggested Fix

Revert the changes in static/app/components/core/button/styles.tsx. The danger case within the getButtonTheme function should use the theme.chonky.embossed.danger tokens for surface, background, and color properties to ensure buttons with priority="danger" are styled correctly.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: static/app/components/core/button/styles.tsx#L284-L286

Potential issue: The code modification in `getButtonTheme` incorrectly maps the 'danger'
button type to use `chonky.embossed.success` theme tokens instead of the correct
`chonky.embossed.danger` tokens. This affects all buttons with `priority="danger"`,
which are used extensively throughout the application. As a result, buttons intended to
warn users about destructive actions will be rendered with green "success" styling,
completely inverting their semantic meaning and potentially misleading users into
performing irreversible actions.

Did we get this right? 👍 / 👎 to inform future reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant