-
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
chore: Dynamic logo, move assets, refactor relationship screen #17494
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request introduces a comprehensive refactoring of logo handling in the financial aid application template. The changes include creating a new Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant LogoComp as Logo Component
participant MunicipalityData as Municipality Data
App->>LogoComp: Pass application object
LogoComp->>MunicipalityData: Extract municipality ID
MunicipalityData-->>LogoComp: Return municipality details
LogoComp->>LogoComp: Dynamically import logo SVG
LogoComp->>App: Render municipality logo
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 ✅ 7 Total Test Services: 0 Failed, 7 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (1)
|
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: 2
🧹 Nitpick comments (17)
libs/application/templates/financial-aid/src/forms/ApplicationForm/index.ts (2)
20-21
: Consider using JSX syntax for creating elements.Instead of importing
createElement
from React and using it to create theLogo
component, you can use JSX syntax for improved readability.Apply this change:
-import { createElement } from 'react' +import React from 'react'Update the
logo
function:-logo: (application: Application) => { - const logo = createElement(Logo, { application }) - return () => logo -}, +logo: (application: Application) => () => <Logo application={application} />,
27-30
: Simplify the 'logo' function definition.You can streamline the
logo
function by returning the component directly without the intermediate variable assignment.Apply this diff to simplify the code:
-logo: (application: Application) => { - const logo = React.createElement(Logo, { application }) - return () => logo -}, +logo: (application: Application) => () => <Logo application={application} />,libs/application/templates/financial-aid/src/forms/MuncipalityNotRegistered.ts (2)
6-7
: Consider using JSX syntax for creating elements.Using JSX syntax can enhance readability by making the code more declarative.
Apply this change:
-import { Logo } from '../components/Logo/Logo' -import { createElement } from 'react' +import React from 'react' +import { Logo } from '../components/Logo/Logo'Update the
logo
function:-logo: (application: Application) => { - const logo = createElement(Logo, { application }) - return () => logo -}, +logo: (application: Application) => () => <Logo application={application} />,
11-14
: Simplify the 'logo' function for clarity.Streamlining the
logo
function improves code readability by removing unnecessary variables.Apply this diff:
-logo: (application: Application) => { - const logo = React.createElement(Logo, { application }) - return () => logo -}, +logo: (application: Application) => () => <Logo application={application} />,libs/application/templates/financial-aid/src/components/Logo/Logo.tsx (1)
28-28
: Add loading and error states to improve UX.The img element should handle loading and error states gracefully.
- return <img src={logo} alt="Municipality logo" /> + return ( + <img + src={logo} + alt="Municipality logo" + onError={(e) => { + e.currentTarget.style.display = 'none' + console.warn('Failed to load municipality logo') + }} + style={{ maxWidth: '100%', height: 'auto' }} + /> + )libs/application/templates/financial-aid/src/fields/ServiceCenter/ServiceCenter.tsx (1)
Line range hint
39-43
: Add security attributes to window.open callAdd
rel="noopener noreferrer"
to prevent potential security vulnerabilities when opening external links.onClick={() => { - window.open(applicantsCenter?.link, '_ blank') + window.open(applicantsCenter?.link, '_blank', 'noopener,noreferrer') }}libs/application/templates/financial-aid/src/forms/ApplicationForm/personalInterestSection/inARelationshipSubsection.ts (1)
13-15
: Improve type safety in condition functionThe current type casting could be made more explicit and type-safe.
-condition: (_, externalData) => - (externalData as unknown as ExternalData).nationalRegistrySpouse.data != - null, +condition: (_: unknown, externalData: unknown) => { + const typedData = externalData as ExternalData + return typedData.nationalRegistrySpouse?.data != null +},libs/application/templates/financial-aid/src/fields/TaxReturnFilesForm/TaxReturnFilesForm.tsx (2)
Line range hint
39-42
: Fix duplicate error messageThe title and message are using the same translation key.
<AlertMessage type="error" title={formatMessage(taxReturnForm.alertMessage.title)} - message={formatMessage(taxReturnForm.alertMessage.title)} + message={formatMessage(taxReturnForm.alertMessage.message)} />
Line range hint
24-27
: Extract complex condition into a helper functionThe complex condition for determining tax data ownership could be extracted into a named helper function for better readability.
+const getTaxDataOwner = (assignees: string[], externalData: ExternalData) => { + const isAssignee = assignees.includes(externalData.nationalRegistry.data.nationalId) + return isAssignee && externalData?.taxDataSpouse?.data + ? externalData.taxDataSpouse.data + : externalData.taxData.data +} const { municipalitiesDirectTaxPayments, municipalitiesPersonalTaxReturn } = - assignees.includes(externalData.nationalRegistry.data.nationalId) && - externalData?.taxDataSpouse?.data - ? externalData.taxDataSpouse.data - : externalData.taxData.data + getTaxDataOwner(assignees, externalData)libs/application/templates/financial-aid/src/forms/SpouseSubmitted.ts (1)
17-20
: Consider using JSX instead of createElement.The logo creation could be simplified using JSX syntax for better readability.
- logo: (application: Application) => { - const logo = createElement(Logo, { application }) - return () => logo - }, + logo: (application: Application) => () => <Logo application={application} />,libs/application/templates/financial-aid/src/forms/ApplicantSubmitted.ts (1)
17-20
: Consider extracting logo factory to a shared utility.The logo implementation is duplicated across multiple forms. Consider creating a shared utility function.
// shared/createFormLogo.ts export const createFormLogo = (application: Application) => () => ( <Logo application={application} /> ) // Usage in forms import { createFormLogo } from '../shared/createFormLogo' export const ApplicantSubmitted: Form = buildForm({ // ... logo: createFormLogo, })libs/application/templates/financial-aid/src/fields/ContactInfo/ContactInfo.tsx (1)
Line range hint
15-15
: Consider using a more specific type for answers.Instead of using type assertion, consider creating a proper type for the answers object structure.
- const { answers } = application + const { answers } = application as { answers: answersSchema }libs/application/templates/financial-aid/src/fields/BankInfoForm/BankInfoForm.tsx (1)
Line range hint
8-8
: Enhance type safety for the answers object.Consider adding proper type definitions for the answers object to improve type safety and maintainability.
- const { answers } = application + const { answers } = application as { answers: { bankInfo?: { bankNumber?: string; ledger?: string; accountNumber?: string } } }libs/application/templates/financial-aid/src/fields/EmploymentForm/EmploymentForm.tsx (1)
Line range hint
13-16
: Consider using a type guard instead of type assertion.Instead of using type assertion for input types, consider creating a proper type guard function.
- const typeInput = { - id: 'employment.type', - error: errors?.employment?.type, - } as InputTypes + const typeInput: InputTypes = { + id: 'employment.type', + error: errors?.employment?.type, + }libs/application/templates/financial-aid/src/forms/PrerequisitesSpouse.ts (1)
27-30
: Consider memoizing the logo component.The current implementation creates a new logo element on every render. Consider memoizing it for better performance.
logo: (application: Application) => { - const logo = createElement(Logo, { application }) - return () => logo + const logo = React.useMemo( + () => createElement(Logo, { application }), + [application] + ) + return () => logo },libs/application/templates/financial-aid/src/forms/ApplicationForm/personalInterestSection/index.ts (1)
33-41
: Refactor duplicated condition logic for children subsections.The condition for checking children custody information is duplicated in two subsections. Consider extracting this logic into a shared function to improve maintainability.
+const hasChildrenWithInfo = (externalData: unknown) => { + const childWithInfo = getValueViaPath( + externalData, + 'childrenCustodyInformation.data', + [], + ) as ApplicantChildCustodyInformation[] + return Boolean(childWithInfo?.length) +} buildSubSection({ - condition: (_, externalData) => { - const childWithInfo = getValueViaPath( - externalData, - 'childrenCustodyInformation.data', - [], - ) as ApplicantChildCustodyInformation[] - return Boolean(childWithInfo?.length) - }, + condition: (_, externalData) => hasChildrenWithInfo(externalData), // ... rest of the code })Also applies to: 53-61
libs/application/templates/financial-aid/src/forms/Prerequisites.ts (1)
34-37
: Enhance type safety in logo implementation.While the logo implementation is functional, consider using JSX syntax for better type checking and readability. The current implementation using
createElement
is less type-safe.- logo: (application: Application) => { - const logo = createElement(Logo, { application }) - return () => logo - }, + logo: (application: Application) => { + return () => <Logo application={application} /> + },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (69)
libs/application/templates/financial-aid/src/assets/svg/akrahreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/akranes.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/akureyri.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/arborg.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/arneshreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/asahreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/blaskogabyggd.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/blonduosbaer.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/bolungarvik.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/borgarbyggd.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/dalabyggd.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/dalvikurbyggd.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/eyja-og-miklaholtshreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/eyjafjardarsveit.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/fjallabyggd.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/fjardabyggd.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/fljotsdalshreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/floahreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/gardabaer.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/grimsnes-og-grafningshreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/grindavikurbaer.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/grundafjardarbaer.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/grytubakkahreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/hafnarfjordur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/helgafellssveit.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/horgarsveit.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/hornafjordur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/hrunamannahreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/hunathing-vestra.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/hunavatnshreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/hvalfjardarsveit.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/hveragerdisbaer.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/isafjardarbaer.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/kaldrananeshreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/kjosarhreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/kopavogur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/langanesbyggd.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/mosfellsbaer.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/mulathing.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/myrdalshreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/nordurthing.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/olfus.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/rangarthing-ytra.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/rangarthing_eystra.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/reykholahreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/reykjanesbaer.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/sambandid.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/seltjarnarnes.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/skaftarhreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/skagabyggd.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/skagafjordur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/skagastrond.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/skeida-og-gnupverjahreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/skorradalur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/skutustadahreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/snaefellsbaer.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/strandabyggd.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/stykkisholmsbaer.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/sudavikurhreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/sudurnesjabaer.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/svalbardshreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/svalbardsstrandarhreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/talknafjardarhreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/thingeyjarsveit.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/tjorneshreppur.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/vestmannaeyjabaer.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/vesturbyggd.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/vogar.svg
is excluded by!**/*.svg
libs/application/templates/financial-aid/src/assets/svg/vopnafjardarhreppur.svg
is excluded by!**/*.svg
📒 Files selected for processing (32)
libs/application/templates/financial-aid/src/components/Logo/Logo.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/AboutForm/AboutForm.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/AboutSpouseForm/AboutSpouseForm.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/BankInfoForm/BankInfoForm.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/ChildrenFilesForm/ChildrenFilesForm.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/ChildrenForm/ChildrenForm.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/ContactInfo/ContactInfo.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/EmploymentForm/EmploymentForm.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/HomeCircumstancesForm/HomeCircumstancesForm.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/InRelationshipForm/InRelationshipForm.tsx
(0 hunks)libs/application/templates/financial-aid/src/fields/IncomeFilesForm/IncomeFilesForm.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/IncomeForm/IncomeForm.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/Logo/Logo.css.ts
(0 hunks)libs/application/templates/financial-aid/src/fields/Logo/Logo.tsx
(0 hunks)libs/application/templates/financial-aid/src/fields/PersonalTaxCreditForm/PersonalTaxCreditForm.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/ServiceCenter/ServiceCenter.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/StudentForm/StudentForm.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/Summary/SpouseSummaryForm.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/Summary/SummaryForm.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/TaxReturnFilesForm/TaxReturnFilesForm.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/UnknownRelationshipForm/UnknownRelationshipForm.tsx
(1 hunks)libs/application/templates/financial-aid/src/fields/index.ts
(0 hunks)libs/application/templates/financial-aid/src/forms/ApplicantSubmitted.ts
(1 hunks)libs/application/templates/financial-aid/src/forms/ApplicationForm/index.ts
(1 hunks)libs/application/templates/financial-aid/src/forms/ApplicationForm/personalInterestSection/inARelationshipSubsection.ts
(1 hunks)libs/application/templates/financial-aid/src/forms/ApplicationForm/personalInterestSection/index.ts
(1 hunks)libs/application/templates/financial-aid/src/forms/MuncipalityNotRegistered.ts
(1 hunks)libs/application/templates/financial-aid/src/forms/Prerequisites.ts
(2 hunks)libs/application/templates/financial-aid/src/forms/PrerequisitesSpouse.ts
(1 hunks)libs/application/templates/financial-aid/src/forms/Spouse.ts
(1 hunks)libs/application/templates/financial-aid/src/forms/SpouseSubmitted.ts
(1 hunks)libs/application/templates/financial-aid/src/lib/FinancialAidTemplate.ts
(1 hunks)
💤 Files with no reviewable changes (4)
- libs/application/templates/financial-aid/src/fields/index.ts
- libs/application/templates/financial-aid/src/fields/Logo/Logo.css.ts
- libs/application/templates/financial-aid/src/fields/InRelationshipForm/InRelationshipForm.tsx
- libs/application/templates/financial-aid/src/fields/Logo/Logo.tsx
✅ Files skipped from review due to trivial changes (1)
- libs/application/templates/financial-aid/src/fields/UnknownRelationshipForm/UnknownRelationshipForm.tsx
🧰 Additional context used
📓 Path-based instructions (27)
libs/application/templates/financial-aid/src/fields/ChildrenFilesForm/ChildrenFilesForm.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/fields/BankInfoForm/BankInfoForm.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/fields/PersonalTaxCreditForm/PersonalTaxCreditForm.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/forms/ApplicantSubmitted.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/fields/IncomeForm/IncomeForm.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/forms/SpouseSubmitted.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/fields/IncomeFilesForm/IncomeFilesForm.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/fields/ServiceCenter/ServiceCenter.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/fields/EmploymentForm/EmploymentForm.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/fields/Summary/SummaryForm.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/fields/ContactInfo/ContactInfo.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/fields/StudentForm/StudentForm.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/fields/AboutForm/AboutForm.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/fields/Summary/SpouseSummaryForm.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/fields/ChildrenForm/ChildrenForm.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/fields/TaxReturnFilesForm/TaxReturnFilesForm.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/forms/PrerequisitesSpouse.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/fields/AboutSpouseForm/AboutSpouseForm.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/forms/Prerequisites.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/forms/ApplicationForm/personalInterestSection/index.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/forms/ApplicationForm/personalInterestSection/inARelationshipSubsection.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/forms/Spouse.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/fields/HomeCircumstancesForm/HomeCircumstancesForm.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/components/Logo/Logo.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/forms/MuncipalityNotRegistered.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/forms/ApplicationForm/index.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/financial-aid/src/lib/FinancialAidTemplate.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🪛 Biome (1.9.4)
libs/application/templates/financial-aid/src/components/Logo/Logo.tsx
[error] 18-18: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (29)
libs/application/templates/financial-aid/src/forms/ApplicationForm/index.ts (2)
13-13
: Importing 'Application' type enhances type safety.Adding the
Application
type to the imports ensures proper typing for thelogo
function parameter, improving type safety and code clarity.
19-19
: Ensure 'personalInterestSection' is correctly integrated.Including
personalInterestSection
in the children array streamlines the form structure. Please verify that all subsections previously defined are now properly encapsulated withinpersonalInterestSection
and that no functionality is lost.libs/application/templates/financial-aid/src/forms/MuncipalityNotRegistered.ts (1)
2-2
: Importing 'Application' type enhances type safety.Including the
Application
type ensures that thelogo
function parameter is properly typed, which aids in maintaining type safety across the application.libs/application/templates/financial-aid/src/fields/ChildrenFilesForm/ChildrenFilesForm.tsx (2)
28-28
: Removal of 'withLogo' HOC is appropriate due to new logo handling.Since the logo is now handled at the form level via the
logo
property inbuildForm
, removing thewithLogo
higher-order component is appropriate and simplifies the component export.
28-28
: Ensure logo display is consistent after HOC removal.Verify that the
ChildrenFilesForm
still displays the logo as intended. With the removal ofwithLogo
, ensure that all instances where this component is used are properly encompassed by forms that provide the logo.libs/application/templates/financial-aid/src/components/Logo/Logo.tsx (1)
7-9
: LGTM! Props are well-typed using TypeScript.The component properly defines its props interface using TypeScript, enhancing type safety and maintainability.
libs/application/templates/financial-aid/src/fields/IncomeFilesForm/IncomeFilesForm.tsx (1)
32-32
: LGTM! Successfully removed withLogo HOC.The removal of the withLogo HOC aligns with the PR objectives of utilizing shared components and improving maintainability.
libs/application/templates/financial-aid/src/fields/AboutForm/AboutForm.tsx (1)
36-36
: LGTM! Successfully removed withLogo HOC.The removal of the withLogo HOC aligns with the PR objectives of utilizing shared components and improving maintainability.
libs/application/templates/financial-aid/src/fields/AboutSpouseForm/AboutSpouseForm.tsx (1)
37-37
: LGTM! Successfully removed withLogo HOC.The removal of the withLogo HOC aligns with the PR objectives of utilizing shared components and improving maintainability.
libs/application/templates/financial-aid/src/fields/ServiceCenter/ServiceCenter.tsx (1)
54-54
: Verify logo visibility after withLogo removalThe removal of withLogo HOC might affect logo visibility in this component.
libs/application/templates/financial-aid/src/fields/PersonalTaxCreditForm/PersonalTaxCreditForm.tsx (1)
55-55
: LGTM! Clean TypeScript implementation using shared componentsThe component effectively uses shared form fields and follows TypeScript best practices.
libs/application/templates/financial-aid/src/fields/IncomeForm/IncomeForm.tsx (1)
60-60
: LGTM! Simplified component export.The removal of the
withLogo
HOC aligns with the PR objectives to reduce custom components and improve maintainability.libs/application/templates/financial-aid/src/forms/SpouseSubmitted.ts (1)
7-7
: LGTM! Proper type imports.The addition of the
Application
type ensures type safety for the logo property.Also applies to: 11-12
libs/application/templates/financial-aid/src/fields/StudentForm/StudentForm.tsx (1)
72-72
: LGTM! Simplified component export.The removal of the
withLogo
HOC aligns with the PR objectives while maintaining component functionality.libs/application/templates/financial-aid/src/fields/ContactInfo/ContactInfo.tsx (1)
67-67
: LGTM! Removal of withLogo HOC aligns with PR objectives.The direct export of the component improves code clarity and reduces unnecessary wrapping.
libs/application/templates/financial-aid/src/fields/BankInfoForm/BankInfoForm.tsx (1)
81-81
: LGTM! Removal of withLogo HOC aligns with PR objectives.The direct export improves code clarity and reduces unnecessary wrapping.
libs/application/templates/financial-aid/src/fields/ChildrenForm/ChildrenForm.tsx (1)
96-96
: LGTM! Clean implementation with proper type safety.The component demonstrates good practices:
- Proper TypeScript usage with FAFieldBaseProps
- Effective use of shared components
- Good error handling and form control
libs/application/templates/financial-aid/src/fields/EmploymentForm/EmploymentForm.tsx (1)
96-96
: LGTM! Removal of withLogo HOC aligns with PR objectives.The direct export improves code clarity and reduces unnecessary wrapping.
libs/application/templates/financial-aid/src/forms/PrerequisitesSpouse.ts (2)
10-15
: LGTM! Clean type imports.The addition of the
Application
type import enhances type safety for the logo property implementation.
20-21
: LGTM! Required imports for logo implementation.The imports of
createElement
andLogo
component are correctly added to support the new logo functionality.libs/application/templates/financial-aid/src/fields/Summary/SpouseSummaryForm.tsx (1)
114-114
: LGTM! Clean removal of withLogo HOC.The removal of the withLogo HOC aligns with the PR's objective to refactor and standardize logo handling at the form level.
libs/application/templates/financial-aid/src/forms/Spouse.ts (3)
8-13
: LGTM! Clean type imports.The addition of the
Application
type import enhances type safety for the logo property implementation.
18-19
: LGTM! Required imports for logo implementation.The imports of
createElement
andLogo
component are correctly added to support the new logo functionality.
25-28
: Consider memoizing the logo component.The current implementation creates a new logo element on every render. Consider memoizing it for better performance.
libs/application/templates/financial-aid/src/fields/HomeCircumstancesForm/HomeCircumstancesForm.tsx (1)
115-115
: LGTM! Clean removal of withLogo HOC.The removal of the withLogo HOC aligns with the PR's objective to refactor and standardize logo handling at the form level.
libs/application/templates/financial-aid/src/forms/ApplicationForm/personalInterestSection/index.ts (1)
1-106
: Well-structured section with proper conditional rendering!The section is well-organized using builder functions and includes appropriate conditional rendering based on external data. The code follows TypeScript best practices and maintains good separation of concerns.
libs/application/templates/financial-aid/src/forms/Prerequisites.ts (1)
10-15
: LGTM! Proper type imports from application types.The imports are well-organized and include all necessary types from the application system.
libs/application/templates/financial-aid/src/fields/Summary/SummaryForm.tsx (1)
206-206
: Clean export without HOC wrapper - good improvement!Removing the
withLogo
HOC simplifies the component structure and follows the PR's objective of standardizing logo handling.libs/application/templates/financial-aid/src/lib/FinancialAidTemplate.ts (1)
112-114
: LGTM! Updated import path matches new file structure.The form loader correctly references the new
ApplicationForm
module, maintaining consistency with the refactored file structure.
const [logo, setLogo] = useState<string>() | ||
const municipality = application.externalData.municipality | ||
?.data as Municipality | ||
|
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.
🛠️ Refactor suggestion
Add type assertion safety checks.
The type assertion for municipality data could be unsafe. Consider adding runtime validation.
- const municipality = application.externalData.municipality
- ?.data as Municipality
+ const municipality = application.externalData.municipality?.data
+ if (municipality && !isMunicipality(municipality)) {
+ console.warn('Invalid municipality data structure')
+ return null
+ }
Consider adding this type guard:
function isMunicipality(data: unknown): data is Municipality {
return (
typeof data === 'object' &&
data !== null &&
'municipalityId' in data
)
}
useEffect(() => { | ||
const getLogo = async () => { | ||
const municipalityId = | ||
municipality && municipality?.municipalityId | ||
? municipality.municipalityId | ||
: '' | ||
const svgLogo = await import( | ||
`../../assets/svg/${logoKeyFromMunicipalityCode[municipalityId]}` | ||
) | ||
setLogo(svgLogo.default) | ||
} | ||
getLogo() | ||
}, [municipality]) |
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.
Add error handling for dynamic imports.
The dynamic import could fail if the logo file doesn't exist. Consider adding error handling.
useEffect(() => {
const getLogo = async () => {
+ try {
const municipalityId =
municipality && municipality?.municipalityId
? municipality.municipalityId
: ''
const svgLogo = await import(
`../../assets/svg/${logoKeyFromMunicipalityCode[municipalityId]}`
)
setLogo(svgLogo.default)
+ } catch (error) {
+ console.error('Failed to load municipality logo:', error)
+ setLogo(undefined)
+ }
}
getLogo()
}, [municipality])
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
useEffect(() => { | |
const getLogo = async () => { | |
const municipalityId = | |
municipality && municipality?.municipalityId | |
? municipality.municipalityId | |
: '' | |
const svgLogo = await import( | |
`../../assets/svg/${logoKeyFromMunicipalityCode[municipalityId]}` | |
) | |
setLogo(svgLogo.default) | |
} | |
getLogo() | |
}, [municipality]) | |
useEffect(() => { | |
const getLogo = async () => { | |
try { | |
const municipalityId = | |
municipality && municipality?.municipalityId | |
? municipality.municipalityId | |
: '' | |
const svgLogo = await import( | |
`../../assets/svg/${logoKeyFromMunicipalityCode[municipalityId]}` | |
) | |
setLogo(svgLogo.default) | |
} catch (error) { | |
console.error('Failed to load municipality logo:', error) | |
setLogo(undefined) | |
} | |
} | |
getLogo() | |
}, [municipality]) |
🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
What
Refactor first screen in the Financial aid application so it uses the application system shared components
Move municipality logos to an assets folder
Make the main form use a dynamic logo
Why
Minimize the use of custom components in the application system, making applications more uniform, predictable and maintainable.
Checklist:
Summary by CodeRabbit
New Features
Logo
component for dynamic logo rendering based on municipality.Component Changes
withLogo
higher-order component from multiple form components.InRelationshipForm
component.Form Enhancements
Code Structure
Application
toApplicationForm
in some modules.