-
Notifications
You must be signed in to change notification settings - Fork 6
Port institutional-partnership page JavaScript files to TypeScript (CORE-1249) #2777
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
Port institutional-partnership page JavaScript files to TypeScript (CORE-1249) #2777
Conversation
7ddb79c
to
2386c5a
Compare
const distributionUrl = 'https://images.openstax.org'; | ||
const version = 'v1'; | ||
|
||
export function maxDimIfNarrowerThan(width: number) { |
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.
This was something duplicated in several modules, so I made it a utility here.
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.
Unfortunately, even the initial conversion came out as delete-and-recreate. :(
return newObject; | ||
} | ||
|
||
function sectionData(data: DataObject, sectionNumber: string | number) { |
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.
This parses out the data for each section, so its output is simply cast to the appropriate props type. We're trusting the CMS to serve the expected format.
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.
Initial conversion did come through as changes, but it's also not a very big file. The only real updates are the added type and the use of the maxDim utility
import type {Icon} from '../participants'; | ||
import './established-partners.scss'; | ||
|
||
export default function EstablishedPartners({model}: {model: Icon[]}) { |
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.
Made this a required parameter
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.
Easier to see changes in individual commits. I made one additional change after the Prettier commit, to handle the possibility that the current/established groups could be empty.
.filter((k) => String(k).startsWith(sectionPrefix)) | ||
.reduce( | ||
(a, oldKey) => unprefixKey(a, String(oldKey), sectionPrefix, data), {} | ||
) |
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.
Do we have other CMS data that needs this transformation? I can see this being a reusable helperDo we have other CMS data that needs this transformation? I can see this being a reusable helper
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.
Somewhat to my surprise, I do not find other pages doing this transformation.
wideCards.map((item) => | ||
<div className="wide-card-entry" key={item}> | ||
wideCards.map((item, index) => | ||
<div className="wide-card-entry" key={index}> |
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.
If these are always in the same order, maybe it's fine, but using the image might be a more stable key
- Converted all 12 JavaScript files in institutional-partnership page to TypeScript - Added comprehensive type definitions following Roy's guidelines: ✅ Avoided using any type ✅ Preferred type to interface ✅ Used type inference where possible ✅ Used inline type definitions for function parameters ✅ Kept line lengths under 120 characters - Enhanced test file with additional sensible tests - Maintained existing functionality while improving type safety Files converted: - institutional-partnership.js → .tsx (DataObject, SectionData, ImageMeta types) - sections/banner/banner.js → .tsx (BannerProps with image meta types) - sections/sign-up/sign-up.js → .tsx (SignUpProps with form fields) - sections/participants/participants.js → .tsx (Icon type, click event handlers) - sections/about/about.js → .tsx (AboutProps with image optimization) - sections/small-quote/small-quote.js → .tsx (SmallQuoteProps with quote fields) - sections/big-quote/big-quote.js → .tsx (BigQuoteProps with background image) - sections/promoting/promoting.js → .tsx (WideCard, TallCard types for complex arrays) - sections/overlapping-quote/overlapping-quote.js → .tsx (OverlappingQuoteProps) - sections/results/results.js → .tsx (Card type for results display) - sections/speaking/speaking.js → .tsx (SpeakingProps with image meta) - sections/participants/established-partners/established-partners.js → .tsx (Icon type) - test/institutional-partnership.test.js → .test.tsx (enhanced with additional tests) 🤖 Generated with [Claude Code](https://claude.ai/code) Lint issues Co-Authored-By: Claude <[email protected]>
6062757
to
f69bb06
Compare
Summary
any
typetype
tointerface
Files Converted
Component Files (12):
institutional-partnership.js
→.tsx
- Main component with DataObject, SectionData, ImageMeta typessections/banner/banner.js
→.tsx
- BannerProps with destructured image meta typessections/sign-up/sign-up.js
→.tsx
- SignUpProps for contact form fieldssections/participants/participants.js
→.tsx
- Icon type, proper click event handlers, groupBy typingsections/about/about.js
→.tsx
- AboutProps with image optimization supportsections/small-quote/small-quote.js
→.tsx
- SmallQuoteProps for quote displaysections/big-quote/big-quote.js
→.tsx
- BigQuoteProps with background image string typesections/promoting/promoting.js
→.tsx
- Complex WideCard, TallCard types for nested arrayssections/overlapping-quote/overlapping-quote.js
→.tsx
- OverlappingQuoteProps for quote sectionsections/results/results.js
→.tsx
- Card type for statistics display with indexed imagessections/speaking/speaking.js
→.tsx
- SpeakingProps with image meta and React inline stylessections/participants/established-partners/established-partners.js
→.tsx
- Icon type for partner displayTest File (1):
test/institutional-partnership.test.js
→.tsx
- Enhanced with additional sensible tests:Technical Details
Test Plan
Related to: https://openstax.atlassian.net/browse/CORE-1249
🤖 Generated with Claude Code