Skip to content

ref(theme): migrate shadows#112997

Open
natemoo-re wants to merge 6 commits intomasterfrom
ref/theme/migrate-shadows
Open

ref(theme): migrate shadows#112997
natemoo-re wants to merge 6 commits intomasterfrom
ref/theme/migrate-shadows

Conversation

@natemoo-re
Copy link
Copy Markdown
Member

a bit more theme cleanup! Replacing the ad-hoc theme.dropShadow* values with our official theme.shadow.* tokens.

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 14, 2026
@sentry

This comment was marked as outdated.

@natemoo-re natemoo-re marked this pull request as ready for review April 15, 2026 20:56
@natemoo-re natemoo-re requested review from a team as code owners April 15, 2026 20:56
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 2 potential issues.

Fix All in Cursor

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

Reviewed by Cursor Bugbot for commit 26c4ba8. Configure here.


&[data-stuck] {
box-shadow: ${p => p.theme.dropShadowHeavyTop};
box-shadow: ${p => p.theme.shadow.high};
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.

Footer shadow direction changed from upward to downward

Medium Severity

The old dropShadowHeavyTop had a negative Y offset (0 -4px 24px ...) casting the shadow upward, which is correct for bottom-positioned sticky/fixed footers. The replacement theme.shadow.high has a positive Y offset (0px 4px 0px 2px ...) casting the shadow downward. For elements pinned to the bottom of the viewport, a downward shadow is invisible and loses the visual separation cue with the content above.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 26c4ba8. Configure here.

const Container = styled('div')`
border: 1px solid ${p => p.theme.tokens.border.primary};
box-shadow: inset ${p => p.theme.dropShadowMedium};
box-shadow: inset ${p => p.theme.shadow.medium};
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.

Inset keyword only applies to first shadow value

Medium Severity

box-shadow: inset ${theme.shadow.medium} prepends inset to a multi-value shadow token. Since shadow.medium expands to two comma-separated shadows, the inset keyword only applies to the first one. The second shadow renders as an external (outset) drop shadow, which differs from the original single inset dropShadowMedium behavior.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 26c4ba8. Configure here.

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