feat: update Icon to be ts and remove all stuff by holaontiveros · Pull Request #4136 · openedx/paragon

holaontiveros · 2026-02-24T21:39:27Z

Description

Converted from JSX to TSX with native TypeScript types
Removed PropTypes in favor of TypeScript interfaces
Replaced defaultProps with default parameter values
Added proper TypeScript interfaces for IconProps and SvgAttrs
Removed obsolete index.d.ts file
Fixed TypeScript compilation errors

Addresses #3744

Deploy Preview

Include a direct link to your changes in this PR's deploy preview here (e.g., a specific component page).

Merge Checklist

Does your change adhere to the documented style conventions?
Were your changes tested using all available themes (see theme switcher in the header of the deploy preview, under the "Settings" icon)?
Is there adequate test coverage for your changes?

Post-merge Checklist

Verify your changes were released to NPM at the expected version.
If you'd like, share your contribution in #show-and-tell.
🎉 🙌 Celebrate! Thanks for your contribution.

openedx-webhooks · 2026-02-24T21:39:32Z

Thanks for the pull request, @holaontiveros!

This repository is currently maintained by @openedx/committers-paragon.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

netlify · 2026-02-24T21:39:32Z

❌ Deploy Preview for paragon-openedx-v23 failed. Why did it fail? →

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`8219ead`
🔍 Latest deploy log	https://app.netlify.com/projects/paragon-openedx-v23/deploys/6a9b3e5aac51050008db05dd

- Converted from JSX to TSX with native TypeScript types - Removed PropTypes in favor of TypeScript interfaces - Replaced defaultProps with default parameter values - Added proper TypeScript interfaces for IconProps and SvgAttrs - Removed obsolete index.d.ts file - Fixed TypeScript compilation errors Co-authored-by: Claude Sonnet 4 <claude@anthropic.com>

codecov · 2026-02-24T22:55:52Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.21%. Comparing base (dfc69e2) to head (8219ead).
⚠️ Report is 62 commits behind head on release-23.x.

Additional details and impacted files

@@               Coverage Diff                @@
##           release-23.x    #4136      +/-   ##
================================================
- Coverage         94.39%   94.21%   -0.18%     
================================================
  Files               242      242              
  Lines              4296     4305       +9     
  Branches           1020      994      -26     
================================================
+ Hits               4055     4056       +1     
- Misses              237      245       +8     
  Partials              4        4

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

mphilbrick211 · 2026-04-06T19:19:47Z

Hi @brian-smith-tcril - can you take a look for us?

bradenmacdonald

Sorry for the delay here @holaontiveros! Can you please get the tests passing and take a look at these two notes? Basically I don't want to change the runtime behavior of className in a TypeScript PR.

Then this looks good to go.

bradenmacdonald · 2026-06-23T01:18:07Z

+   * the `id` property of the Icon element, by default this value is generated
+   * with the `newId` function with the `prefix` of `Icon`.
+   */
+  id?: string | null;


Where did null come from? It seems like id just accepted string | undefined before, so can we leave it at that?

bradenmacdonald · 2026-06-23T01:19:53Z

+  /** The size of the icon. */
+  size?: 'xs' | 'sm' | 'md' | 'lg' | 'inline';
+  /** A class name that will define what the Icon looks like. */
+  className?: string | string[];


I think our Paragon components generally just accept className?: string, so I don't think we should be adding new string[] functionality here (nor the corresponding Array.isArray(className) ? className.join(' ') : code below).

Fine to do that in a separate PR if you'd like, but I think it's better for className to behave consistently across all Paragon components.

mphilbrick211 · 2026-07-13T16:10:32Z

Hi @holaontiveros! Friendly ping on this.

bradenmacdonald · 2026-08-04T21:14:44Z

@holaontiveros Are you planning to finish up this PR? It's the last thing blocking me from marking #3744 as complete, so I'd like to get it done one way or another. Thanks :)

bradenmacdonald · 2026-09-04T17:59:06Z

@jacobo-dominguez-wgu Any chance you could check with @holaontiveros and help us get this PR over the line? I'd like to close out #3744 and this is the last thing blocking it.

holaontiveros · 2026-09-04T19:23:02Z

@jacobo-dominguez-wgu Any chance you could check with @holaontiveros and help us get this PR over the line? I'd like to close out #3744 and this is the last thing blocking it.

Let me work on in, I'll take just some mins I just forgot to follow this one, Jacobo did ping, thanks

bradenmacdonald · 2026-09-04T19:28:32Z

Thanks!

- Revert id type to string (drop the null it never had before) - Narrow className to string for consistency with other Paragon components; remove the Array.isArray(...).join(' ') handling - Update Icon.test.tsx to match the narrowed types (string className, drop the obsolete null-id test)

The strict ComponentType<SVGAttributes<SVGElement>> broke Alert, which passes icon?: React.ComponentType<IconProps> to <Icon src=...>. Match the IconButton convention (iconAs?: React.ComponentType<any>) so any icon component is accepted while still rejecting non-component values like strings.

holaontiveros · 2026-09-04T21:06:17Z

@bradenmacdonald I think it's ready but Netlify seemed to failed momentarily I can't re-trigger those could you?

bradenmacdonald · 2026-09-04T21:10:32Z

@holaontiveros Thanks a lot! I'll take a look soon. I checked, but I can't find any way to trigger the Netlify job again. Perhaps try rebasing / squashing?

I'll just note for myself that your changes are 9da500f and 618c4f1 so I can see the changes even if you squash them.

holaontiveros · 2026-09-04T21:58:19Z

@bradenmacdonald I did a small change just to re-trigger the netlify stuff but still fails, did something change with netlify related configs?

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Feb 24, 2026

openedx-webhooks added this to Contributions Feb 24, 2026

github-project-automation Bot moved this to Needs Triage in Contributions Feb 24, 2026

holaontiveros force-pushed the feat/migrate-ts-icon branch from 250ce89 to 2c45014 Compare February 24, 2026 21:41

holaontiveros requested a review from brian-smith-tcril February 24, 2026 21:49

mphilbrick211 added the mao-onboarding Reviewing this will help onboard devs from an Axim mission-aligned organization (MAO). label Feb 24, 2026

mphilbrick211 moved this from Needs Triage to Ready for Review in Contributions Feb 24, 2026

bradenmacdonald reviewed Apr 13, 2026

View reviewed changes

Comment thread src/Icon/Icon.test.tsx

bradenmacdonald mentioned this pull request Apr 13, 2026

Tracking issue: remove propTypes and defaultProps from TypeScript components #3744

Open

15 tasks

mphilbrick211 moved this from Ready for Review to In Eng Review in Contributions Apr 29, 2026

chore: pushed a bit of missing code

ac59684

openedx-webhooks added the core contributor PR author is a Core Contributor (who may or may not have write access to this repo). label May 26, 2026

bradenmacdonald reviewed Jun 23, 2026

View reviewed changes

itsjeyd added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Jun 25, 2026

holaontiveros added 2 commits September 4, 2026 13:51

docs: tweak Icon src prop comment to re-trigger deploy preview

8219ead

Uh oh!

Conversation

holaontiveros commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Deploy Preview

Merge Checklist

Post-merge Checklist

Uh oh!

openedx-webhooks commented Feb 24, 2026

Uh oh!

netlify Bot commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ Deploy Preview for paragon-openedx-v23 failed. Why did it fail? →

Uh oh!

codecov Bot commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mphilbrick211 commented Apr 6, 2026

Uh oh!

Uh oh!

bradenmacdonald left a comment

Choose a reason for hiding this comment

Uh oh!

bradenmacdonald Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

bradenmacdonald Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

mphilbrick211 commented Jul 13, 2026

Uh oh!

bradenmacdonald commented Aug 4, 2026

Uh oh!

bradenmacdonald commented Sep 4, 2026

Uh oh!

holaontiveros commented Sep 4, 2026

Uh oh!

bradenmacdonald commented Sep 4, 2026

Uh oh!

holaontiveros commented Sep 4, 2026

Uh oh!

bradenmacdonald commented Sep 4, 2026

Uh oh!

holaontiveros commented Sep 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

holaontiveros commented Feb 24, 2026 •

edited

Loading

netlify Bot commented Feb 24, 2026 •

edited

Loading

codecov Bot commented Feb 24, 2026 •

edited

Loading