Tech Debt - Continue removing makeStyles part 2 #1722
Conversation
rdonigian
force-pushed
the
remove-makeStyles
branch
from
26aa528 to
6ef6c66
Compare
I did have the UI pulled up along side while making the changes. Thanks for the the review. |
📝 WalkthroughWalkthroughThis change refactors multiple React components to remove the use of the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/components/ProjectChangeRequestListItem/ProjectChangeRequestListItem.tsx (2)
2-2: LGTM! Correctly implements past review feedback.The Box component usage for the dash separator matches the previous review suggestion exactly, with proper text color and horizontal margin styling.
Also applies to: 53-53
79-84: LGTM! Correctly implements past review feedback.The CardActions styling matches the previous review suggestion exactly, using flex with space-between and proper theme-based padding.
src/components/Upload/UploadItem.tsx (2)
85-95: Inconsistent color for completed state IconButton.The completed state IconButton uses
color="error"but based on past review feedback, this should becolor="primary"to properly indicate a successful completion state.Apply this fix:
<IconButton aria-label="completed" - color="error" + color="primary" onClick={() => console.log('TODO: Add onCompleted click handler')} >
97-108: LGTM! Progress container styling matches past feedback.The Box component with
p: '12px'correctly implements the padding as suggested in past review comments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/components/Changeset/ChangesetBanner.tsx(1 hunks)src/components/Debug/Code.tsx(2 hunks)src/components/Error/Error.tsx(3 hunks)src/components/FieldOverviewCard/FieldOverviewCard.tsx(4 hunks)src/components/FundingAccountCard/FundingAccountCard.tsx(1 hunks)src/components/LanguageEngagementListItemCard/LanguageEngagementListItemCard.tsx(4 hunks)src/components/LocationCard/LocationCard.tsx(3 hunks)src/components/MemberListSummary/MemberListSummary.tsx(2 hunks)src/components/MethodologiesCard/MethodologiesCard.tsx(3 hunks)src/components/OrganizationListItemCard/OrganizationListItemCard.tsx(1 hunks)src/components/PartnerListItemCard/PartnerListItemCard.tsx(2 hunks)src/components/ProgressButton/ProgressButton.tsx(1 hunks)src/components/ProjectChangeRequestListItem/ProjectChangeRequestListItem.tsx(4 hunks)src/components/Redacted/Redacted.tsx(2 hunks)src/components/Upload/UploadItem.tsx(2 hunks)src/components/Upload/UploadItems.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: CarsonF
PR: SeedCompany/cord-field#1604
File: src/components/Feature.tsx:12-19
Timestamp: 2024-10-21T15:17:35.618Z
Learning: In `src/components/Feature.tsx`, the `Feature` component uses `useFeatureFlagPayload` and the `match` prop to determine feature availability, which differs from the `useFeatureEnabled` hook.
src/components/Changeset/ChangesetBanner.tsx (2)
Learnt from: CarsonF
PR: #1604
File: src/components/Feature.tsx:12-19
Timestamp: 2024-10-21T15:17:35.618Z
Learning: In src/components/Feature.tsx, the Feature component uses useFeatureFlagPayload and the match prop to determine feature availability, which differs from the useFeatureEnabled hook.
Learnt from: CarsonF
PR: #1606
File: src/api/client/ImpersonationContext.tsx:70-83
Timestamp: 2024-10-22T13:07:22.162Z
Learning: In the file src/api/client/ImpersonationContext.tsx, within the ImpersonationProvider component, the broadcast variable initialized with useState remains constant and does not need to be included in the dependency array of useEffect hooks that reference it.
src/components/Upload/UploadItems.tsx (2)
Learnt from: CarsonF
PR: #1606
File: src/api/client/ImpersonationContext.tsx:70-83
Timestamp: 2024-10-22T13:07:22.162Z
Learning: In the file src/api/client/ImpersonationContext.tsx, within the ImpersonationProvider component, the broadcast variable initialized with useState remains constant and does not need to be included in the dependency array of useEffect hooks that reference it.
Learnt from: CarsonF
PR: #1604
File: src/components/Feature.tsx:12-19
Timestamp: 2024-10-21T15:17:35.618Z
Learning: In src/components/Feature.tsx, the Feature component uses useFeatureFlagPayload and the match prop to determine feature availability, which differs from the useFeatureEnabled hook.
src/components/ProjectChangeRequestListItem/ProjectChangeRequestListItem.tsx (1)
Learnt from: CarsonF
PR: #1604
File: src/components/Feature.tsx:12-19
Timestamp: 2024-10-21T15:17:35.618Z
Learning: In src/components/Feature.tsx, the Feature component uses useFeatureFlagPayload and the match prop to determine feature availability, which differs from the useFeatureEnabled hook.
src/components/FundingAccountCard/FundingAccountCard.tsx (2)
Learnt from: CarsonF
PR: #1604
File: src/components/Feature.tsx:12-19
Timestamp: 2024-10-21T15:17:35.618Z
Learning: In src/components/Feature.tsx, the Feature component uses useFeatureFlagPayload and the match prop to determine feature availability, which differs from the useFeatureEnabled hook.
Learnt from: CarsonF
PR: #1606
File: src/api/client/ImpersonationContext.tsx:70-83
Timestamp: 2024-10-22T13:07:22.162Z
Learning: In the file src/api/client/ImpersonationContext.tsx, within the ImpersonationProvider component, the broadcast variable initialized with useState remains constant and does not need to be included in the dependency array of useEffect hooks that reference it.
src/components/Error/Error.tsx (2)
Learnt from: CarsonF
PR: #1604
File: src/components/Feature.tsx:12-19
Timestamp: 2024-10-21T15:17:35.618Z
Learning: In src/components/Feature.tsx, the Feature component uses useFeatureFlagPayload and the match prop to determine feature availability, which differs from the useFeatureEnabled hook.
Learnt from: CarsonF
PR: #1696
File: src/scenes/FieldZones/CreateFieldZone/CreateFieldZone.tsx:0-0
Timestamp: 2025-06-20T17:50:15.534Z
Learning: In this codebase, GraphQL documents (like CreateFieldZoneDocument, CreateFieldRegionDocument, etc.) are already properly typed, and Apollo hooks (useMutation, useQuery, etc.) automatically infer the correct types from these typed documents. Therefore, explicit generic type parameters on Apollo hooks are unnecessary and should not be suggested.
src/components/MemberListSummary/MemberListSummary.tsx (1)
Learnt from: CarsonF
PR: #1606
File: src/api/client/ImpersonationContext.tsx:70-83
Timestamp: 2024-10-22T13:07:22.162Z
Learning: In the file src/api/client/ImpersonationContext.tsx, within the ImpersonationProvider component, the broadcast variable initialized with useState remains constant and does not need to be included in the dependency array of useEffect hooks that reference it.
src/components/LocationCard/LocationCard.tsx (1)
Learnt from: CarsonF
PR: #1696
File: src/scenes/FieldZones/CreateFieldZone/CreateFieldZone.tsx:0-0
Timestamp: 2025-06-20T17:50:15.534Z
Learning: In this codebase, GraphQL documents (like CreateFieldZoneDocument, CreateFieldRegionDocument, etc.) are already properly typed, and Apollo hooks (useMutation, useQuery, etc.) automatically infer the correct types from these typed documents. Therefore, explicit generic type parameters on Apollo hooks are unnecessary and should not be suggested.
src/components/FieldOverviewCard/FieldOverviewCard.tsx (1)
Learnt from: CarsonF
PR: #1604
File: src/components/Feature.tsx:12-19
Timestamp: 2024-10-21T15:17:35.618Z
Learning: In src/components/Feature.tsx, the Feature component uses useFeatureFlagPayload and the match prop to determine feature availability, which differs from the useFeatureEnabled hook.
src/components/Upload/UploadItem.tsx (1)
Learnt from: CarsonF
PR: #1604
File: src/components/Feature.tsx:12-19
Timestamp: 2024-10-21T15:17:35.618Z
Learning: In src/components/Feature.tsx, the Feature component uses useFeatureFlagPayload and the match prop to determine feature availability, which differs from the useFeatureEnabled hook.
src/components/PartnerListItemCard/PartnerListItemCard.tsx (1)
Learnt from: CarsonF
PR: #1604
File: src/components/Feature.tsx:12-19
Timestamp: 2024-10-21T15:17:35.618Z
Learning: In src/components/Feature.tsx, the Feature component uses useFeatureFlagPayload and the match prop to determine feature availability, which differs from the useFeatureEnabled hook.
🧬 Code Graph Analysis (9)
src/components/Redacted/Redacted.tsx (1)
src/common/sx.ts (1)
extendSx(19-22)
src/components/OrganizationListItemCard/OrganizationListItemCard.tsx (1)
src/components/Routing/CardActionAreaLink.tsx (1)
CardActionAreaLink(20-34)
src/components/FundingAccountCard/FundingAccountCard.tsx (1)
src/components/DisplaySimpleProperty/DisplaySimpleProperty.tsx (1)
DisplaySimpleProperty(14-50)
src/components/Error/Error.tsx (1)
src/components/Routing/Navigate.tsx (2)
StatusCode(59-65)statusCode(89-91)
src/components/Debug/Code.tsx (1)
src/common/sx.ts (1)
extendSx(19-22)
src/components/FieldOverviewCard/FieldOverviewCard.tsx (4)
src/common/CalenderDate.ts (1)
DateTimeOrISO(20-20)src/components/Routing/CardActionAreaLink.tsx (1)
CardActionAreaLink(20-34)src/components/Routing/ButtonLink.tsx (1)
ButtonLink(26-39)src/common/sx.ts (1)
extendSx(19-22)
src/components/Upload/UploadItem.tsx (1)
src/components/IconButton/IconButton.tsx (1)
IconButton(12-29)
src/components/PartnerListItemCard/PartnerListItemCard.tsx (1)
src/components/Routing/CardActionAreaLink.tsx (1)
CardActionAreaLink(20-34)
src/components/LanguageEngagementListItemCard/LanguageEngagementListItemCard.tsx (2)
src/components/Routing/CardActionAreaLink.tsx (1)
CardActionAreaLink(20-34)src/components/Changeset/useChangesetAwareIdFromUrl.ts (1)
idForUrl(28-29)
🔇 Additional comments (44)
src/components/ProgressButton/ProgressButton.tsx (1)
35-38: Explanatory comment adds valuable context – no action needed.Clear, succinct, and directly next to the relevant code. 👍
src/components/Upload/UploadItems.tsx (1)
46-48: LGTM! Clean migration from makeStyles to sx prop.The styling change correctly applies the disabled action color using MUI's sx prop with theme access. This is a proper replacement for the previous makeStyles approach.
src/components/Changeset/ChangesetBanner.tsx (1)
23-26: LGTM! Proper sx prop implementation with theme access.The styling correctly uses theme breakpoints and spacing functions. This is a clean migration from makeStyles to inline sx styling.
src/components/Redacted/Redacted.tsx (1)
3-3: LGTM! Correctly implements previous review feedback.The changes properly address the past review comments by:
- Using
text: 'initial'to target the text slot instead of transform- Using
extendSxutility to safely handle the potentially array/function sx prop- Converting to array-based sx composition for proper style merging
Also applies to: 25-30
src/components/ProjectChangeRequestListItem/ProjectChangeRequestListItem.tsx (1)
36-42: LGTM! Proper conditional styling for loading state.The conditional sx prop correctly applies flex display only during loading state, maintaining proper layout behavior.
src/components/FundingAccountCard/FundingAccountCard.tsx (4)
11-11: Note: Component marked for potential removal.The comment indicates this component is a candidate for removal, which provides useful context for future refactoring efforts.
17-17: LGTM! Proper card sizing with responsive width.The Card styling correctly sets full width with a reasonable maximum width constraint.
19-19: LGTM! Clean spacing implementation.The Typography styling uses proper MUI spacing shorthand for consistent margin bottom.
27-27: LGTM! Proper action alignment.The CardActions styling correctly aligns content to the right, which is standard for card action layouts.
src/components/Error/Error.tsx (4)
3-3: LGTM: Clean theme import addition.The
useThemeimport is appropriately added to support the inline styling migration.
56-56: LGTM: Theme hook integration.The theme hook is correctly initialized and will provide access to MUI theme values for the conditional styling.
80-89: LGTM: Proper conditional styling implementation.The conditional styling logic correctly applies page-specific styles when the
pageprop is true, using theme spacing values for consistent spacing. The migration from makeStyles to inline styles maintains the same functionality.
96-96: LGTM: Consistent sx prop migration.The Grid container correctly migrates from class-based styling to the
sxprop withmt: 3for margin-top, maintaining the original spacing behavior.src/components/MemberListSummary/MemberListSummary.tsx (3)
58-58: LGTM: Appropriate inline styling for layout.The inline style with flexbox properties correctly replaces the makeStyles implementation for the container layout.
59-59: LGTM: Clean sx prop migration for spacing.The AvatarGroup correctly uses the
sxprop withmr: 1for right margin, maintaining the original spacing behavior.
71-71: LGTM: Proper flex styling migration.The Typography component correctly uses the
sxprop withflexGrow: 1to fill available space, preserving the original layout behavior.src/components/OrganizationListItemCard/OrganizationListItemCard.tsx (3)
19-25: LGTM: Proper responsive styling with theme integration.The Card component correctly uses the
sxprop with a theme function to access breakpoint values, maintaining responsive behavior while removing the makeStyles dependency.
29-29: LGTM: Clean flex layout styling.The CardActionAreaLink correctly applies flex display with
alignItems: 'initial'to maintain the original layout behavior.
31-39: LGTM: Comprehensive layout styling migration.The CardContent component properly migrates to the
sxprop with all necessary layout properties (flex,py,px,display,justifyContent) to maintain the original card layout and spacing.src/components/LocationCard/LocationCard.tsx (3)
33-33: LGTM: Clean width constraint styling.The Card component correctly uses the
sxprop to set width constraints (width: '100%', maxWidth: 400), maintaining the original responsive behavior.
62-62: LGTM: Proper layout styling for actions.The CardActions component correctly uses the
sxprop withjustifyContent: 'space-between'to maintain the original button layout.
79-82: LGTM: Clean padding migration.The Typography component properly migrates to the
sxprop withpr: 1for right padding, maintaining the original spacing behavior.src/components/LanguageEngagementListItemCard/LanguageEngagementListItemCard.tsx (6)
6-6: LGTM: Appropriate Stack import.The Stack component import is correctly added to replace the div wrapper in the layout, providing better semantic structure.
37-37: LGTM: Clean card width styling.The Card component correctly uses the
sxprop to set full width, maintaining responsive behavior.
40-44: LGTM: Proper flex layout for clickable area.The CardActionAreaLink correctly applies flex display with
alignItems: 'initial'to maintain the original card layout structure.
45-52: LGTM: Comprehensive content area styling.The CardContent component properly migrates all layout properties (
flex,px,py,display) to thesxprop, maintaining the original card content layout and spacing.
58-58: LGTM: Grid flex styling migration.The Grid component correctly uses the
sxprop withflex: 1to maintain the original layout behavior.
84-101: LGTM: Excellent div-to-Stack migration.The replacement of the generic div with a Stack component is a good improvement. The
sxprop correctly applies all necessary layout properties (flex,textAlign,ml,justifyContent) to maintain the original right-aligned content area layout.src/components/Upload/UploadItem.tsx (3)
5-11: LGTM! Import changes support the styling migration.The added imports for
BoxandStackcomponents are appropriate for replacing the previous class-based layout with MUI layout components.
32-43: LGTM! Container styling properly migrated to sx prop.The Box component correctly implements the flexbox layout with theme-aware spacing and divider styling. The conditional border styling using
:not(:last-of-type)is appropriate.
44-83: LGTM! Stack and Typography styling correctly implemented.The Stack component provides proper spacing and the Typography components handle text overflow correctly with
textOverflow: 'ellipsis'andwhiteSpace: 'nowrap'. The conditional color handling for error states is appropriate.src/components/PartnerListItemCard/PartnerListItemCard.tsx (3)
26-33: LGTM! Card styling properly migrated with theme-aware breakpoints.The Card component correctly uses theme breakpoints for responsive max-width and sets up relative positioning for the absolutely positioned toggle button.
37-37: LGTM! CardActionAreaLink flex styling is appropriate.The flex display with
alignItems: 'initial'correctly maintains the card's layout structure.
65-65: LGTM! TogglePinButton positioning is correctly implemented.The absolute positioning with
top: 5, right: 5appropriately places the toggle button in the top-right corner of the card.src/components/Debug/Code.tsx (1)
3-3: LGTM! ExtendSx import supports proper style composition.The addition of
extendSximport is necessary for the new style composition approach used in the component.src/components/MethodologiesCard/MethodologiesCard.tsx (4)
34-44: LGTM! Grid item styling correctly implements flex layout.The Grid components properly use flex layout with appropriate color theming and spacing. The icon container correctly uses
text.secondarycolor and margin.
60-68: LGTM! CardContent styling properly implements full height layout.The CardContent component correctly uses flex column layout with full height and appropriate padding values.
85-85: LGTM! Card sizing is correctly implemented.The Card uses
width: '1', height: '1'which correctly implements full width and height. Note that numeric valueswidth: 1, height: 1would be equivalent.
87-87: LGTM! CardActionArea height styling is appropriate.The CardActionArea correctly uses full height styling to match the parent Card component.
src/components/FieldOverviewCard/FieldOverviewCard.tsx (5)
6-6: LGTM! Import changes and CardProps addition properly support styling migration.The additions of
CardProps,Stack, andextendSximports are appropriate for the new styling approach. The removal of the duplicateDateTimeOrISOimport resolves the redundancy. AddingCardPropsto the component interface enables external style overrides as requested in past feedback.Also applies to: 9-9, 16-16, 27-28, 40-40, 57-57
64-75: LGTM! Card styling properly implements style composition.The Card component correctly uses the
sxarray syntax with base flex layout styles and merges external styles viaextendSx(CardProps?.sx). This addresses the past feedback about forwardingsxstyles to the Card component.
79-86: LGTM! ActionArea styling correctly implements flex layout.The ActionArea component properly uses flex layout with appropriate justification and padding values using theme spacing.
89-105: LGTM! Stack component and Typography styling correctly implemented.The Stack component properly replaces the previous layout with appropriate spacing and alignment. The conditional Typography color styling using
sxprop correctly handles the empty value state withaction.disabledcolor.
113-118: LGTM! Grid styling in CardActions is properly implemented.The Grid container correctly uses
sxprop for padding, alignment, and space distribution, effectively replacing the previous class-based styling.
rdonigian
force-pushed
the
remove-makeStyles
branch
from
6ef6c66 to
79f4218
Compare
This is a continuation of removing the deprecated makeStyles from the codebase. Targeting src/components as the first folder to purge.