Skip to content

feat: migrate RadioIcon to Blend Design System#4640

Open
kanikabansal08 wants to merge 3 commits intoblend-checkbox-migratefrom
blend-radio-migrate
Open

feat: migrate RadioIcon to Blend Design System#4640
kanikabansal08 wants to merge 3 commits intoblend-checkbox-migratefrom
blend-radio-migrate

Conversation

@kanikabansal08
Copy link
Copy Markdown
Collaborator

@kanikabansal08 kanikabansal08 commented Mar 31, 2026

Closes #4639

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

Migrates RadioIcon (custom SVG radio indicator) to the Blend Design System. Creates RadioBinding.res (Blend Radio + RadioGroup binding) and RadioIconAdapter.res (feature-flagged adapter). All 7 <RadioIcon call sites updated to <RadioIconAdapter; RadioIcon.res untouched.

YesNoRadioInput.res has been removed — it had zero call sites in the codebase and was dead code.

Motivation and Context

Continuation of the incremental Blend migration chain (blend-filters-migrateblend-checkbox-migrate → this PR). Unlike CheckBoxIcon, Blend's Radio is a native HTML input that does not stop click propagation — parent div onClick handlers fire naturally, so no call-site restructuring was required for the parent-div pattern.

How did you test it?

Tested manually with devBlendEnabled: true in config.toml.

UI screens to verify:

Screen What to check
Developer → Payment Settings Radio options under each section toggle (e.g. Capture Method) — Blend Radio renders
Developer → Payment Settings (Revamped) Same, new design
Connectors → Additional Details Sidebar Selected payment experience radio indicator (visual only)
Filter/SelectBox single-select dropdowns Legacy fallback — visually unchanged

Where to test it?

  • INTEG
  • SANDBOX
  • PROD

Checklist

  • I ran npm run re:build
  • I reviewed submitted code
  • I added unit tests for my changes where possible

@kanikabansal08 kanikabansal08 requested a review from a team as a code owner March 31, 2026 09:52
@semanticdiff-com
Copy link
Copy Markdown

Review changes with  SemanticDiff

@kanikabansal08 kanikabansal08 self-assigned this Mar 31, 2026
YesNoRadioInput has zero call sites in the codebase — dead code removed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@kanikabansal08 kanikabansal08 changed the title feat: migrate RadioIcon and YesNoRadioInput to Blend Design System feat: migrate RadioIcon to Blend Design System Mar 31, 2026
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@muditbhutani
Copy link
Copy Markdown
Collaborator

@product-manager what do you think

@hs-control-center-bot
Copy link
Copy Markdown
Collaborator

Product Manager Analysis (powered by Claude)

Analysis: Migrate RadioIcon to Blend Design System (#4640)

This is a clean, well-scoped incremental step in the Blend Design System migration chain (blend-filters-migrateblend-checkbox-migrateblend-radio-migrate). The PR does exactly what it says — wraps RadioIcon in a feature-flagged adapter and swaps all 7 call sites.

What's done well

  • Adapter pattern is consistent with the prior CheckBoxIconAdapter migration — contributors familiar with that PR will immediately understand this one.
  • No call-site restructuring needed because Blend's Radio doesn't stop click propagation (unlike Checkbox). The PR description correctly calls this out, which is helpful context for reviewers.
  • Dead code removal of YesNoRadioInput.res is a good housekeeping move — zero callers, no reason to keep it.
  • Incidental cleanup: migrating React.useContext(BlendContext.blendEnabledContext)BlendContext.useBlendEnabled() across the other adapters is a minor but positive consistency fix.

Concerns

1. Hardcoded fill color in the adapter (RadioIconAdapter.res:8)

The default fill="#0EB025" is a raw hex value. Several call sites pass theme-aware values like fill={textColor.primaryNormal} or fill="text-nd_primary_blue-450", which is correct. But the fallback default bakes in a specific green that may not match all themes. This is inherited from the original RadioIcon defaults, but worth flagging — if Blend's Radio handles its own color via the design system, the fill prop becomes irrelevant in the Blend path, which is fine. Just confirm the legacy path default is intentional.

2. RadioGroup binding is unused

RadioBinding.res defines both Radio and RadioGroup, but only Radio is consumed. The RadioGroup binding was likely intended for the YesNoRadioInput rewrite mentioned in issue #4639, but that component was deleted instead. This is fine as forward-looking scaffolding if RadioGroup will be used in a future PR — but if not, it's dead code on arrival. Clarify intent.

3. Issue #4639 describes a different scope than what shipped

The issue says YesNoRadioInput would get an "A1 rewrite" (internals rewritten with Blend RadioGroup). Instead, the PR just deletes it. The deletion is justified (zero callers), but the issue should be updated to reflect the actual approach so it doesn't confuse future contributors reading the migration history.

4. Missing unit tests

The checklist shows "I added unit tests" is unchecked. For an adapter that maps prop types between two component APIs (especially the size mapping Small → Small, Large → Medium), a basic snapshot or render test would catch regressions if either the legacy or Blend component API changes. This is low-risk but worth adding given it's a pattern that will repeat for future migrations.

Questions

  • Is there a tracking issue or checklist for the overall Blend migration? Knowing what's left (e.g., remaining components still using legacy SVG patterns) would help prioritize.
  • The size mapping maps Large → Medium — is this intentional because Blend doesn't have a Large radio variant, or should a Large Blend size be requested from the design system team?

Next steps

  1. Update issue feat: migrate RadioIcon and YesNoRadioInput to Blend Design System #4639 to reflect that YesNoRadioInput was deleted (not rewritten) and explain why.
  2. Decide on RadioGroup binding — keep if planned for near-term use, remove if not.
  3. Add a basic render test for RadioIconAdapter covering both Blend-enabled and legacy paths.
  4. Manual QA on the four screens listed, with devBlendEnabled: true and false, to confirm visual parity.

Overall this is a low-risk, well-executed migration step. Ship it once the minor items above are addressed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: migrate RadioIcon and YesNoRadioInput to Blend Design System

3 participants