-
Notifications
You must be signed in to change notification settings - Fork 698
Fix: Restore Heading component secondary color styling (regression in v10.2.2) #1358
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
Conversation
🦋 Changeset detectedLatest commit: e49d253 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Changed Heading from extending Text with attrs to being a standalone styled.div with its own attrs. This fixes the issue where attrs from parent components were overriding child component defaults. Co-authored-by: ryan-roemer <445557+ryan-roemer@users.noreply.github.com>
@copilot Please add a changeset to this PR at |
Co-authored-by: ryan-roemer <445557+ryan-roemer@users.noreply.github.com>
Added changeset at patch level in commit |
"spectacle": patch | ||
--- | ||
|
||
Fix Heading component not applying secondary color styling. Restores pink color (#fc6986) for headings that was lost in v10.2.2 when replacing defaultProps with attrs method. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot this should be a named color (primary
, secondary
or whatever) and not refer to hex code or color name (e.g. pink). Rest looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated changeset to refer to "secondary color" and "correct theme color" instead of hex code or color name in commit 6d7af9b.
Co-authored-by: ryan-roemer <445557+ryan-roemer@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚢 🚢 🚢
Summary
Fixed a regression in v10.2.2 where
Heading
components lost their pink/secondary color styling. The issue was introduced in PR #1351 when replacing ReactdefaultProps
with styled-components.attrs()
method for React 19 compatibility.Problem
#ebe5da
) instead of secondary color (pink#fc6986
)Root Cause
When a styled component with
.attrs()
extends another styled component with.attrs()
, the parent's attrs output becomes the child's attrs input. This caused Text'scolor: 'primary'
default to override Heading'scolor: 'secondary'
setting.Additionally, the original fix used an empty object
({})
instead ofcompose(color, typography, space)
, which prevented styled-system from resolving theme values.Solution
Changed
Heading
fromstyled(Text).attrs(...)
tostyled.div.attrs(...)
to be a standalone component. This ensures:Changes
Heading
component inpackages/spectacle/src/components/typography.tsx
Testing
Screenshots
After fix - Headings properly display in pink/secondary color:
Changeset
Added changeset at patch level documenting this fix for the next release.
Original prompt
Fixes #1357
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.