-
Notifications
You must be signed in to change notification settings - Fork 61
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
feat(web): Pension Calculator - Display icon to left of title for 2025 preview #17251
feat(web): Pension Calculator - Display icon to left of title for 2025 preview #17251
Conversation
WalkthroughThe pull request introduces a new Changes
Sequence DiagramsequenceDiagram
participant PC as PensionCalculator
participant PCT as PensionCalculatorTitle
participant Intl as react-intl
PC->>PCT: Render with props
PCT->>Intl: Fetch translations
alt isNewSystemActive is true
PCT-->>PC: Render title with image
else
PCT-->>PC: Render simple title
end
Possibly related PRs
Suggested labels
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
Datadog ReportAll test runs ✅ 4 Total Test Services: 0 Failed, 4 Passed Test Services
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17251 +/- ##
=======================================
Coverage 35.73% 35.73%
=======================================
Files 6937 6938 +1
Lines 148315 148305 -10
Branches 42331 42318 -13
=======================================
Hits 52999 52999
+ Misses 95316 95306 -10
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/web/screens/Organization/SocialInsuranceAdministration/translationStrings.ts (1)
628-634
: Consider managing image URLs through a configuration file.While the implementation works, hardcoding CDN URLs in translation strings makes it difficult to update them if needed. Consider:
- Moving image URLs to a separate configuration file
- Using the configuration values in translation strings
Example approach:
// config/images.ts export const images = { pensionCalculator: { after1stSeptember2025Icon: 'https://images.ctfassets.net/8k0h54kbe6bj/5RIwKVet87Nm4ycltkzjnX/9c594855a9b2f90dde63766ee87a09ca/58dd40fbf365769d984be22a9b64bc29.png' } } // translationStrings.ts import { images } from '../config/images' export const translationStrings = defineMessages({ after1stSeptember2025IconUrl: { id: 'web.pensionCalculator:after1stSeptember2025IconUrl', defaultMessage: images.pensionCalculator.after1stSeptember2025Icon, description: 'Mynd vinstra megin við titil "Reiknivél örorku- og endurhæfingargreiðslna eftir 1. september 2025"' } })apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.css.ts (1)
50-52
: LGTM! Consider adding a comment for clarity.The
noWrap
style correctly prevents flex items from wrapping, which is essential for maintaining the icon-title layout.export const noWrap = style({ + // Prevents wrapping of flex items (used for icon-title layout) flexWrap: 'nowrap', })
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculatorTitle.tsx (1)
26-31
: Consider adding image dimensions to CSS.While the width and height attributes are good for preventing layout shifts, consider moving these dimensions to CSS for better maintainability.
+const ICON_DIMENSIONS = { + width: 84, + height: 70, +} as const; <img - width={84} - height={70} + width={ICON_DIMENSIONS.width} + height={ICON_DIMENSIONS.height} src={formatMessage(translationStrings.after1stSeptember2025IconUrl)} alt="" />
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.css.ts
(1 hunks)apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx
(3 hunks)apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculatorResults.tsx
(3 hunks)apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculatorTitle.tsx
(1 hunks)apps/web/screens/Organization/SocialInsuranceAdministration/translationStrings.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.css.ts (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/Organization/SocialInsuranceAdministration/translationStrings.ts (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculatorTitle.tsx (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculatorResults.tsx (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (3)
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx (1)
53-53
: LGTM! Clean refactoring of the title component.
The changes effectively:
- Extract title rendering logic into a dedicated component
- Maintain consistent prop usage across both instances
- Improve code organization and reusability
Also applies to: 558-563, 584-589
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculatorTitle.tsx (1)
8-13
: LGTM! Props interface is well-defined.
The interface clearly defines the required props with appropriate types.
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculatorResults.tsx (1)
365-370
: LGTM! Clean integration of PensionCalculatorTitle component.
The new component is consistently used in both locations where the title needs to be rendered, improving code organization and maintainability.
Also applies to: 391-396
Pension Calculator - Display icon to left of title for 2025 preview
Why
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
New Features
PensionCalculatorTitle
component for improved title rendering.noWrap
to enhance layout styling.Bug Fixes
PensionCalculatorResults
component.