Skip to content

fix(feedback): Deep merge custom styles with defaults in FeedbackWidget#5625

Merged
antonis merged 1 commit intogetsentry:mainfrom
sbs44:fix/feedback-widget-deep-merge-styles
Feb 6, 2026
Merged

fix(feedback): Deep merge custom styles with defaults in FeedbackWidget#5625
antonis merged 1 commit intogetsentry:mainfrom
sbs44:fix/feedback-widget-deep-merge-styles

Conversation

@sbs44
Copy link
Contributor

@sbs44 sbs44 commented Feb 6, 2026

📢 Type of change

  • Bugfix

📜 Description

Replace the shallow spread in FeedbackWidget.render() with a dynamic deep merge that iterates over all default style keys. Custom style properties now override individual values while preserving unspecified defaults.

Before (shallow — replaces entire style object):

const styles = { ...defaultStyles(theme), ...this.props.styles };

After (deep — merges per key):

const styles = Object.fromEntries(
  Object.keys(_defaultStyles).map(key => [
    key,
    { ..._defaultStyles[key], ..._propStyles[key] },
  ]),
) as FeedbackWidgetStyles;

💡 Motivation and Context

Fixes #5624 — passing partial style overrides (e.g. styles={{ input: { color: 'red' } }}) currently loses all default properties for that key (height, padding, borderWidth, etc.), breaking layout.

💚 How did you test it?

  • Added unit test: render with partial input style override, assert default properties (height, borderWidth, borderRadius, paddingHorizontal, marginBottom, fontSize) are preserved
  • Updated existing snapshots to reflect the corrected merge behavior
  • All existing tests pass

📝 Checklist

  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • All tests passing
  • No breaking changes

🔮 Next steps

None — this is a self-contained fix.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2026

Semver Impact of This PR

None (no version bump detected)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


  • fix(feedback): Deep merge custom styles with defaults in FeedbackWidget by sbs44 in #5625

🤖 This preview updates automatically when you update the PR.

@sbs44 sbs44 force-pushed the fix/feedback-widget-deep-merge-styles branch 2 times, most recently from 61eb5a1 to b389cde Compare February 6, 2026 11:44
@lucas-zimerman lucas-zimerman added the ready-to-merge Triggers the full CI test suite label Feb 6, 2026
@lucas-zimerman
Copy link
Collaborator

The PR is good, and I'd say it's ready to merge. I just want more opinions from other reviewers if we should consider this a break change, despite being a bugfix.

@lucas-zimerman
Copy link
Collaborator

@sentry review

@antonis
Copy link
Contributor

antonis commented Feb 6, 2026

The PR is good, and I'd say it's ready to merge. I just want more opinions from other reviewers if we should consider this a break change, despite being a bugfix.

That's a good point @lucas-zimerman 👍
I think the majority of the users won't face any issues since to overcome the bug/shortcoming of the implementation they had to add all properties. I would suggest just adding an extra note on the changelog like #5625 (comment)
Wdyt?

- Replace shallow spread with dynamic deep merge over all style keys
- Custom style props now override specific properties while preserving defaults
- Add test verifying partial style overrides don't lose default properties
- Update snapshots to reflect correct merged styles
@sbs44 sbs44 force-pushed the fix/feedback-widget-deep-merge-styles branch from b389cde to cad207f Compare February 6, 2026 12:36
Copy link
Collaborator

@lucas-zimerman lucas-zimerman left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for your contribution!

Copy link
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

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

LGTM 🎸
Thank you for your contribution 🙇

@antonis antonis merged commit a483f9f into getsentry:main Feb 6, 2026
81 of 82 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Triggers the full CI test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FeedbackWidget: custom styles prop replaces defaults instead of merging

3 participants