feat: convert ProgressBar component to TypeScript#4139
Conversation
Replace PropTypes and defaultProps with TypeScript interfaces and parameter defaults to support React 19 migration. Resolves openedx#3739. - Rename .jsx/.js files to .tsx/.ts - Add ProgressBarAnnotatedProps interface with JSDoc comments - Type refs, utility functions, and component props - Remove ts-ignore comment from src/index.ts - Export ProgressBarAnnotatedProps type for consumers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thanks for the pull request, @brianjbuck-wgu! This repository is currently maintained by 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 approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo 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:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere 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:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
✅ Deploy Preview for paragon-openedx-v23 ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
openedx-webhooks
added
the
open-source-contribution
Claude Code Session Summary — Feb 24, 2026ObjectiveConvert the Work Completed1. Converted
|
mphilbrick211
added
the
mao-onboarding
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release-23.x #4139 +/- ##
================================================
+ Coverage 94.39% 94.40% +0.01%
================================================
Files 242 242
Lines 4296 4305 +9
Branches 1020 1022 +2
================================================
+ Hits 4055 4064 +9
Misses 237 237
Partials 4 4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @openedx/committers-paragon! Would someone be able to take a look at this for us? Thanks! |
|
Friendly ping on this, @openedx/committers-paragon :) |
Kelketek
left a comment
Kelketek
left a comment
There was a problem hiding this comment.
Thanks for this work, @brianjbuck-wgu . Had a few notes for you.
| }); | ||
| it('should apply styles based on direction for threshold', () => { | ||
| window.getComputedStyle = jest.fn().mockReturnValue({ getPropertyValue: () => 'rtl' }); | ||
| window.getComputedStyle = jest.fn().mockReturnValue({ getPropertyValue: () => 'rtl' }) as any; |
There was a problem hiding this comment.
Can you use jest.spyOn with the mock implementation instead of assigning this property and casting it to any?
|
|
||
| expect(computedStyles.getPropertyValue('directory')).toBe('rtl'); | ||
| window.getComputedStyle.mockRestore(); | ||
| (window.getComputedStyle as jest.Mock).mockRestore(); |
There was a problem hiding this comment.
The return value of spyOn should be the right type.
| ], | ||
| }, | ||
| }; | ||
| } as any; |
There was a problem hiding this comment.
This structure is used in two places but is cast to any. Can we find a way to type it properly so it's used correctly in both places?
Other concern-- this isn't part of your PR but seems like something we should clean up while we're here:
The object is created once but is used in multiple tests. That means it could change between tests and any subtle errors from this would be difficult to detect. Could you create a function which returns this object instead and use that for each test?
| ], | ||
| }, | ||
| }; | ||
| } as any; |
There was a problem hiding this comment.
We've got two files where this type is used. Could we get a type annotation here? Also noticed that this is a bare object but should be created via function (see my other comment on a similar line)
| it('returns false if reference is wrong', () => { | ||
| const wrongRef1 = {}; | ||
| const wrongRef2 = { current: {} }; | ||
| const wrongRef1 = {} as any; |
There was a problem hiding this comment.
I'm guessing these are any because we're literally testing garbage data? OK if so.
4 participants
Description
Convert the
ProgressBarcomponent from JavaScript (.jsx) to TypeScript (.tsx) as part of the ongoing migration tracked in #3739.This change eliminates deprecated React APIs (
PropTypesanddefaultProps) to support the eventual React 19 upgrade and provides build-time type safety.Changes:
index.jsx→index.tsx,utils.js→utils.ts, and test files to.tsx/.tsPropTypeswithProgressBarAnnotatedPropsTypeScript interface (with JSDoc comments preserving prop descriptions)defaultPropswith parameter defaults in the function signatureuseRefcalls, utility functions, and component props// @ts-ignorecomment fromsrc/index.tsProgressBarAnnotatedPropstype for consumersDeploy Preview
Check the ProgressBar component page in the deploy preview to verify props documentation renders correctly.
Merge Checklist
exampleapp?Post-merge Checklist