feat(application-system): Add locale to arguments passed to options functions for select, radio and checkbox fields #16644
Conversation
… and checkbox fields
WalkthroughThe changes in this pull request primarily involve modifications to several files related to localization enhancements. A new type, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
|
norda-gunni
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
libs/application/ui-fields/src/lib/SelectFormField/SelectFormField.tsx (1)
Line range hint 1-94: Component follows reusability guidelines
The SelectFormField component is well-structured for reuse:
- Props are properly typed with TypeScript
- Component is framework-agnostic (works with any NextJS app)
- Dependencies are properly imported from shared packages
libs/application/templates/reference-template/src/forms/ExampleForm.ts (1)
119-136: Consider adding TypeScript type safety.
The locale parameter lacks type safety. Consider adding proper TypeScript types to ensure only valid locales are passed.
Add type definition:
type SupportedLocale = 'is' | 'en'
options: (options: unknown, application: unknown, locale: SupportedLocale) => {
// ... rest of the implementation
}libs/application/types/src/lib/Fields.ts (1)
29-31: Well-structured approach to internationalization.
The consistent implementation of locale support across field types (Checkbox, Radio, Select) through the new MaybeWithApplicationAndFieldAndLocale type provides a robust foundation for multilingual form fields. This approach:
- Maintains type safety
- Allows both static and dynamic options
- Provides a consistent API across different field types
Also applies to: 280-280, 317-317, 330-330
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
- libs/application/core/src/lib/fieldBuilders.ts (2 hunks)
- libs/application/templates/reference-template/src/forms/ExampleForm.ts (1 hunks)
- libs/application/types/src/lib/Fields.ts (4 hunks)
- libs/application/ui-fields/src/lib/CheckboxFormField/CheckboxFormField.tsx (1 hunks)
- libs/application/ui-fields/src/lib/RadioFormField/RadioFormField.tsx (1 hunks)
- libs/application/ui-fields/src/lib/SelectFormField/SelectFormField.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
libs/application/core/src/lib/fieldBuilders.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/reference-template/src/forms/ExampleForm.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/types/src/lib/Fields.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/ui-fields/src/lib/CheckboxFormField/CheckboxFormField.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/ui-fields/src/lib/RadioFormField/RadioFormField.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/ui-fields/src/lib/SelectFormField/SelectFormField.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."
🔇 Additional comments (11)
libs/application/ui-fields/src/lib/SelectFormField/SelectFormField.tsx (1)
39-39: LGTM: Locale destructuring is properly implemented
The addition of locale destructuring from useLocale hook is clean and follows TypeScript conventions.
libs/application/ui-fields/src/lib/CheckboxFormField/CheckboxFormField.tsx (2)
43-43: LGTM: Clean locale extraction
The destructuring of lang as locale from the useLocale hook is well-implemented and follows TypeScript best practices.
46-47: LGTM: Proper locale integration with options building
The locale parameter is correctly integrated into buildFieldOptions and the useMemo dependency array is properly updated to include locale.
Let's verify the locale parameter usage in buildFieldOptions:
✅ Verification successful
Locale parameter is correctly implemented and used
The verification confirms that:
buildFieldOptionsfunction inlibs/application/core/src/lib/fieldBuilders.tscorrectly accepts the locale parameter in its signature- The function is properly called with the locale parameter in CheckboxFormField, RadioFormField, and SelectFormField components
- The dependency array in the CheckboxFormField's useMemo hook correctly includes the locale parameter
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify buildFieldOptions implementation accepts locale parameter
# Expected: Should find the updated function signature with locale parameter
ast-grep --pattern 'buildFieldOptions = ($params) => {
$$$
}'
Length of output: 65
Script:
#!/bin/bash
# Let's try a different approach to find buildFieldOptions
# First, let's find the file containing buildFieldOptions
rg "buildFieldOptions" -l
# Then let's see the implementation with context
rg "buildFieldOptions" -A 10
Length of output: 7802
libs/application/ui-fields/src/lib/RadioFormField/RadioFormField.tsx (2)
42-42: LGTM: Clean locale integration
The destructuring of lang as locale from useLocale hook maintains consistent naming with the field builder interface while preserving type safety.
45-46: Verify locale propagation in buildFieldOptions
The integration looks good, with proper dependency tracking in useMemo. Let's verify the locale propagation through buildFieldOptions.
✅ Verification successful
Locale parameter is correctly implemented in buildFieldOptions
The implementation in fieldBuilders.ts shows that buildFieldOptions properly handles the locale parameter:
- It accepts locale as the fourth parameter
- The locale is correctly passed to the options function when options is a function:
maybeOptions(application, field, locale) - The function signature uses the correct type
MaybeWithApplicationAndFieldAndLocale<Option[]>
The changes in the review are consistent with the implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that buildFieldOptions is consistently using locale parameter across the codebase
ast-grep --pattern 'buildFieldOptions($$$locale$$$)'
Length of output: 54
Script:
#!/bin/bash
# Let's first find the buildFieldOptions function definition
ast-grep --pattern 'function buildFieldOptions($_) {
$$$
}'
# Also search for any direct usage of buildFieldOptions
rg "buildFieldOptions" -A 3
Length of output: 3014
Script:
#!/bin/bash
# Let's check the implementation of buildFieldOptions in fieldBuilders.ts
cat libs/application/core/src/lib/fieldBuilders.ts
Length of output: 21099
libs/application/core/src/lib/fieldBuilders.ts (2)
47-47: LGTM! Clean import additions.
The new imports are properly organized and correctly sourced from their respective modules.
Also applies to: 49-49
529-535: Verify the impact on existing option functions.
The changes look good and maintain backward compatibility. However, let's verify that all existing option functions in the codebase have been updated to handle the new locale parameter.
libs/application/types/src/lib/Fields.ts (4)
29-31: LGTM! Well-designed type definition.
The new type MaybeWithApplicationAndFieldAndLocale<T> follows TypeScript best practices and maintains consistency with existing patterns while adding locale support.
330-330: Verify the options function implementation for SelectField.
The type change enables locale-based options, but we should verify that all implementations properly handle the new locale parameter.
#!/bin/bash
# Find all implementations of SelectField options
ast-grep --pattern 'options: ($value) => {
$$$
}'280-280: Verify the options function implementation for CheckboxField.
The type change enables locale-based options, but we should verify that all implementations properly handle the new locale parameter.
✅ Verification successful
The previous pattern was too specific. Let's try a broader search to find any options implementations and CheckboxField references to understand the usage patterns.
Based on the search results, I can now generate a final response regarding the CheckboxField options type change:
The type change to include locale support is correctly implemented and used consistently
The search results show that the CheckboxField interface in libs/application/types/src/lib/Fields.ts is the central definition used across the codebase. All implementations using CheckboxField properly handle options through the buildCheckboxField builder function, which automatically handles the locale-aware type. No direct usage of the raw interface was found that would bypass the builder pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all implementations of CheckboxField options
ast-grep --pattern 'options: ($value) => {
$$$
}'
Length of output: 53
Script:
#!/bin/bash
# Search for CheckboxField options implementations with broader pattern
ast-grep --pattern 'options: $$$'
# Also search for direct references to CheckboxField
rg -A 5 "CheckboxField"
Length of output: 98994
317-317: Verify the options function implementation for RadioField.
The type change enables locale-based options, but we should verify that all implementations properly handle the new locale parameter.
✅ Verification successful
Radio field implementations properly handle locale parameter
Based on the codebase analysis, I can see that radio field implementations are properly handling the locale parameter through the form builders and component system:
- The
buildRadioFieldfunction inlibs/application/core/src/lib/fieldBuilders.tscorrectly constructs radio fields with all necessary properties - Radio field implementations in various forms (e.g., health insurance, parental leave) use the form builder system that inherently supports localization
- The component mapper and form rendering system properly handle translations and locale-specific content through the
formatTextandformatMessageutilities
The type change to MaybeWithApplicationAndFieldAndLocale<Option[]> is consistent with the implementation patterns found in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all implementations of RadioField options
ast-grep --pattern 'options: ($value) => {
$$$
}'
Length of output: 53
Script:
#!/bin/bash
# Search for RadioField options implementations with broader patterns
rg -l "RadioField.*options" --type ts
# Also search for any options property assignments
ast-grep --pattern 'options = $_'
# Look for direct options property definitions
ast-grep --pattern 'options: $_'
# Search for any radio-related option implementations
rg -A 5 "radio.*options" --type ts
Length of output: 4531
Script:
#!/bin/bash
# Search for RadioField type usage and its implementations
rg -A 10 "RadioField.*=" --type ts
# Search for radio field definitions in form schemas
rg -A 10 "component.*radio" --type ts
# Look for radio field configurations
rg -A 10 "type.*radio" -g "*.ts"
Length of output: 28083
Datadog ReportAll test runs ✅ 21 Total Test Services: 0 Failed, 20 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (18)
|
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Outside diff range and nitpick comments (18)
libs/application/templates/funding-government-projects/src/fields/FieldTitle/FieldTitle.tsx (1)
Line range hint 40-42: Consider using formatTextWithLocale for field.description
For consistency with the title formatting, consider using formatTextWithLocale for the description as well.
{field.description && (
- <Text>{formatText(field.description, application, formatMessage)}</Text>
+ <Text>{formatTextWithLocale(field.description, application, locale as Locale, formatMessage)}</Text>
)}libs/application/ui-fields/src/lib/LinkFormField/LinkFormField.tsx (1)
15-15: Consider moving type cast to destructuring
The type cast could be moved to the destructuring for better type safety throughout the component.
-const { formatMessage, lang: locale } = useLocale()
+const { formatMessage, lang: locale }: { formatMessage: any, lang: Locale } = useLocale()libs/application/ui-shell/src/hooks/useApplicationTitle.ts (2)
14-15: Remove redundant type assertion
The locale parameter is already typed as Locale in the function signature, making the type assertion on line 38 unnecessary.
- locale as Locale,
+ locale,Also applies to: 35-39
49-54: Improve type safety and naming consistency
Two suggestions for improvement:
- The type assertion
locale as Localeis unnecessary ifuseLocalehook is properly typed - Consider keeping the original variable name
langfor clarity, or update theuseLocalehook to returnlocaledirectly
- const { formatMessage, lang: locale } = useLocale()
- const applicationTitle = getApplicationTitle(
- state,
- locale as Locale,
- formatMessage,
- )
+ const { formatMessage, lang } = useLocale()
+ const applicationTitle = getApplicationTitle(
+ state,
+ lang,
+ formatMessage,
+ )libs/application/ui-fields/src/lib/AccordionFormField/AccordionFormField.tsx (1)
19-19: Consider explicit type annotation for locale
While the type is inferred from useLocale, consider adding an explicit type annotation for better type safety and documentation:
- const { formatMessage, lang: locale } = useLocale()
+ const { formatMessage, lang: locale }: { formatMessage: MessageFormatter; lang: string } = useLocale()libs/application/ui-fields/src/lib/AlertMessageFormField/AlertMessageFormField.tsx (1)
Line range hint 1-67: Architecture maintains reusability standards
The changes preserve the component's reusability while enhancing localization capabilities. The component:
- Remains framework-agnostic
- Uses TypeScript effectively
- Maintains clear separation of concerns
libs/application/templates/operating-license/src/fields/PropertyOverviewRepeater/index.tsx (2)
20-20: Consider using explicit type for locale.
While the code works, consider explicitly typing the locale value from useLocale for better type safety.
- const { formatMessage, lang: locale } = useLocale()
+ const { formatMessage, lang: locale }: { formatMessage: any, lang: Locale } = useLocale()Line range hint 13-17: Consider enhancing component reusability with prop validation.
While the component is well-structured, consider adding:
- Default props
- Prop validation for required fields
- Error handling for missing or invalid field data
Example implementation:
interface PropTypes extends FieldBaseProps {
field: CustomField
onError?: (error: Error) => void
defaultTitle?: string
}
PropertyOverviewRepeater.defaultProps = {
defaultTitle: '',
onError: console.error,
}Also applies to: 42-48
libs/application/ui-fields/src/lib/CheckboxFormField/CheckboxFormField.tsx (1)
47-49: Consider memoization strategy for locale-dependent options
The current implementation will trigger a re-render of all options when the locale changes. For large option lists, consider:
- Memoizing the options transformation separately from the build step
- Using a locale-keyed cache if the options are frequently accessed
libs/application/ui-fields/src/lib/BankAccountFormField/BankAccountFormField.tsx (1)
Line range hint 20-31: Ensure consistent usage of locale-aware formatting
While the title now uses formatTextWithLocale, the other text formatting calls (bankNumber, ledger, accountNumber) still use formatText. For consistency and proper localization support, consider updating these calls as well.
Apply this pattern to the other formatText calls:
- const bankNumber = formatText(
+ const bankNumber = formatTextWithLocale(
coreDefaultFieldMessages.defaultBankAccountBankNumber,
application,
+ locale as Locale,
formatMessage,
)libs/application/templates/grindavik-housing-buyout/src/fields/AdditionalOwnersOverview/index.tsx (1)
46-51: Consider making the formatTextWithLocale call more concise
While the implementation is correct, the function call could be more concise while maintaining readability.
- {formatTextWithLocale(
- field.title,
- application,
- locale as Locale,
- formatMessage,
- )}
+ {formatTextWithLocale(field.title, application, locale as Locale, formatMessage)}libs/application/templates/announcement-of-death/src/fields/FilesRecipientCard/index.tsx (1)
Line range hint 31-42: Consider extracting options generation logic to a separate function.
The options generation logic could be moved to a separate function to improve readability and maintainability. This would also make it easier to test and potentially reuse across similar components.
Consider refactoring like this:
const getEstateMembers = (members: Array<Member>) =>
members
?.filter((member) => !member?.dummy)
.map((estateMember) => ({
label: estateMember.name,
value: estateMember.nationalId,
}))
.filter((member) => isPerson(member.value)) ?? []
// In component
let options = application.answers?.estateMembers?.members?.length
? getEstateMembers(application.answers.estateMembers.members)
: []libs/application/ui-shell/src/components/FormStepper.tsx (1)
61-66: Consider improving type safety
The type assertion locale as Locale suggests that type inference isn't working optimally. Consider updating the useLocale hook's return type to properly type lang as Locale, eliminating the need for type assertion.
libs/application/ui-fields/src/lib/DateFormField/DateFormField.tsx (1)
Line range hint 91-94: Consider using formatTextWithLocale consistently for all text elements.
For consistency with the label localization, consider updating the description and placeholder formatting to also use formatTextWithLocale. This would ensure uniform localization support across all text elements in the component.
{description && (
<FieldDescription
- description={formatText(description, application, formatMessage)}
+ description={formatTextWithLocale(description, application, lang, formatMessage)}
/>
)}
<Box paddingTop={2}>
<DatePickerController
// ... other props ...
placeholder={
placeholder
- ? formatText(placeholder, application, formatMessage)
+ ? formatTextWithLocale(placeholder, application, lang, formatMessage)
: undefined
}Also applies to: 139-143
libs/application/templates/data-protection-complaint/src/fields/CommissionFieldRepeater/CommissionFieldRepeater.tsx (2)
30-30: Consider adding type annotation for useLocale destructuring
While functional, we can improve type safety by explicitly typing the destructured values.
- const { formatMessage, lang: locale } = useLocale()
+ const { formatMessage, lang: locale }: { formatMessage: TFunction; lang: Locale } = useLocale()Line range hint 1-124: Consider updating remaining formatText calls for consistency
While the title now uses locale-aware formatting, there are several other formatText calls in the component (for labels and button text) that might benefit from the same locale-aware treatment using formatTextWithLocale for consistency.
// Example for one of the labels:
-label={formatText(
- info.labels.name,
- application,
- formatMessage,
-)}
+label={formatTextWithLocale(
+ info.labels.name,
+ application,
+ locale as Locale,
+ formatMessage,
+)}libs/application/templates/operating-license/src/fields/PropertyRepeater/index.tsx (2)
36-36: Consider explicit typing for locale.
While the code works, consider explicitly typing the destructured values for better type safety:
- const { formatMessage, lang: locale } = useLocale()
+ const { formatMessage, lang: locale }: { formatMessage: (msg: any) => string, lang: Locale } = useLocale()Line range hint 31-54: Consider extracting locale-aware field components.
The PropertyRepeater component could benefit from further modularization. Consider extracting the locale-aware field handling into a separate hook or utility function that could be reused across different templates.
This would:
- Improve reusability across NextJS apps
- Make testing easier
- Reduce code duplication for similar locale-aware fields
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (35)
- libs/application/core/src/lib/formUtils.ts (2 hunks)
- libs/application/templates/announcement-of-death/src/fields/FilesRecipientCard/index.tsx (4 hunks)
- libs/application/templates/data-protection-complaint/src/fields/CommissionFieldRepeater/CommissionFieldRepeater.tsx (4 hunks)
- libs/application/templates/data-protection-complaint/src/fields/FieldLabel.tsx (1 hunks)
- libs/application/templates/funding-government-projects/src/fields/FieldTitle/FieldTitle.tsx (2 hunks)
- libs/application/templates/grindavik-housing-buyout/src/fields/AdditionalOwnersOverview/index.tsx (2 hunks)
- libs/application/templates/health-insurance/src/fields/TextWithTooltip/TextWithTooltip.tsx (2 hunks)
- libs/application/templates/institution-collaboration/src/fields/SecondaryContact/SecondaryContact.tsx (2 hunks)
- libs/application/templates/operating-license/src/fields/PropertyOverviewRepeater/index.tsx (3 hunks)
- libs/application/templates/operating-license/src/fields/PropertyRepeater/index.tsx (2 hunks)
- libs/application/templates/reference-template/src/forms/ExampleForm.ts (3 hunks)
- libs/application/types/src/lib/Fields.ts (6 hunks)
- libs/application/types/src/lib/Form.ts (3 hunks)
- libs/application/ui-fields/src/lib/AccordionFormField/AccordionFormField.tsx (2 hunks)
- libs/application/ui-fields/src/lib/AlertMessageFormField/AlertMessageFormField.tsx (2 hunks)
- libs/application/ui-fields/src/lib/AsyncSelectFormField/AsyncSelectFormField.tsx (4 hunks)
- libs/application/ui-fields/src/lib/BankAccountFormField/BankAccountFormField.tsx (2 hunks)
- libs/application/ui-fields/src/lib/CheckboxFormField/CheckboxFormField.tsx (3 hunks)
- libs/application/ui-fields/src/lib/CompanySearchFormField/CompanySearchFormField.tsx (3 hunks)
- libs/application/ui-fields/src/lib/DateFormField/DateFormField.tsx (3 hunks)
- libs/application/ui-fields/src/lib/DescriptionFormField/DescriptionFormField.tsx (4 hunks)
- libs/application/ui-fields/src/lib/DividerFormField/DividerFormField.tsx (1 hunks)
- libs/application/ui-fields/src/lib/ExpandableDescriptionFormField/ExpandableDescriptionFormField.tsx (2 hunks)
- libs/application/ui-fields/src/lib/ImageFormField/ImageFormField.tsx (3 hunks)
- libs/application/ui-fields/src/lib/LinkFormField/LinkFormField.tsx (2 hunks)
- libs/application/ui-fields/src/lib/PhoneFormField/PhoneFormField.tsx (4 hunks)
- libs/application/ui-fields/src/lib/RadioFormField/RadioFormField.tsx (4 hunks)
- libs/application/ui-fields/src/lib/SelectFormField/SelectFormField.tsx (4 hunks)
- libs/application/ui-fields/src/lib/StaticTableFormField/StaticTableFormField.tsx (3 hunks)
- libs/application/ui-fields/src/lib/SubmitFormField/SubmitFormField.tsx (3 hunks)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (4 hunks)
- libs/application/ui-fields/src/lib/TextFormField/TextFormField.tsx (4 hunks)
- libs/application/ui-shell/src/components/FormStepper.tsx (4 hunks)
- libs/application/ui-shell/src/components/Screen.tsx (3 hunks)
- libs/application/ui-shell/src/hooks/useApplicationTitle.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/application/templates/reference-template/src/forms/ExampleForm.ts
- libs/application/ui-fields/src/lib/RadioFormField/RadioFormField.tsx
- libs/application/ui-fields/src/lib/SelectFormField/SelectFormField.tsx
🧰 Additional context used
📓 Path-based instructions (32)
libs/application/core/src/lib/formUtils.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/announcement-of-death/src/fields/FilesRecipientCard/index.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/data-protection-complaint/src/fields/CommissionFieldRepeater/CommissionFieldRepeater.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/data-protection-complaint/src/fields/FieldLabel.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/funding-government-projects/src/fields/FieldTitle/FieldTitle.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/grindavik-housing-buyout/src/fields/AdditionalOwnersOverview/index.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/health-insurance/src/fields/TextWithTooltip/TextWithTooltip.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/institution-collaboration/src/fields/SecondaryContact/SecondaryContact.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/operating-license/src/fields/PropertyOverviewRepeater/index.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/operating-license/src/fields/PropertyRepeater/index.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/types/src/lib/Fields.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/types/src/lib/Form.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/ui-fields/src/lib/AccordionFormField/AccordionFormField.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/ui-fields/src/lib/AlertMessageFormField/AlertMessageFormField.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/ui-fields/src/lib/AsyncSelectFormField/AsyncSelectFormField.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/ui-fields/src/lib/BankAccountFormField/BankAccountFormField.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/ui-fields/src/lib/CheckboxFormField/CheckboxFormField.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/ui-fields/src/lib/CompanySearchFormField/CompanySearchFormField.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/ui-fields/src/lib/DateFormField/DateFormField.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/ui-fields/src/lib/DescriptionFormField/DescriptionFormField.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/ui-fields/src/lib/DividerFormField/DividerFormField.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/ui-fields/src/lib/ExpandableDescriptionFormField/ExpandableDescriptionFormField.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/ui-fields/src/lib/ImageFormField/ImageFormField.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/ui-fields/src/lib/LinkFormField/LinkFormField.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/ui-fields/src/lib/PhoneFormField/PhoneFormField.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/ui-fields/src/lib/StaticTableFormField/StaticTableFormField.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/ui-fields/src/lib/SubmitFormField/SubmitFormField.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/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.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/ui-fields/src/lib/TextFormField/TextFormField.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/ui-shell/src/components/FormStepper.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/ui-shell/src/components/Screen.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/ui-shell/src/hooks/useApplicationTitle.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."
🔇 Additional comments (83)
libs/application/templates/data-protection-complaint/src/fields/FieldLabel.tsx (2)
1-7: LGTM! Imports are well-structured and promote reusability.
The imports are properly organized, using absolute paths and TypeScript types from shared libraries, which aligns with the coding guidelines for reusability across NextJS apps.
13-22: Consider improving type safety by updating useLocale hook types.
The explicit type cast locale as Locale suggests a potential type mismatch between the lang property from useLocale and the required Locale type. Consider updating the useLocale hook's type definitions instead of using type assertion.
Let's verify the type definitions:
libs/application/ui-fields/src/lib/DividerFormField/DividerFormField.tsx (4)
3-3: LGTM: Import changes look good!
The imports are well-organized and properly typed, following TypeScript best practices.
Also applies to: 7-7
15-15: LGTM: Hook usage is clean and clear!
The destructuring syntax is well-used, and the renaming from lang to locale improves code clarity.
Line range hint 1-38: LGTM: Code meets all guidelines for library components!
The component:
- Is fully reusable across NextJS apps
- Uses TypeScript appropriately for type safety
- Follows proper bundling practices
20-25: 🛠️ Refactor suggestion
Consider improving type safety for locale parameter.
While the code works, the type casting locale as Locale suggests that the type system isn't properly inferring the type. This could be improved for better type safety.
Let's verify the type definitions:
Consider updating the useLocale hook's type definitions to properly type the lang property as Locale, eliminating the need for type casting:
interface UseLocaleReturn {
formatMessage: MessageFormatter;
lang: Locale; // Instead of string
// ... other properties
}✅ Verification successful
Type casting is appropriate in this case
The type casting locale as Locale is actually necessary and correct here. Looking at the implementation of formatTextWithLocale, we can see that:
- The function explicitly expects a
Localetype parameter - The
Localetype is imported from '@island.is/shared/types' and is used as a strict type parameter - The locale parameter is used in function calls that require the specific
Localetype
The type casting ensures type safety by guaranteeing that the locale value conforms to the expected Locale type definition. This is a common pattern when working with values that come from external sources or hooks where the type system cannot automatically infer the specific type.
The original review comment's concern about type safety can be dismissed as the type casting is being used appropriately in this context.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the useLocale hook's return type definition
rg -t typescript "export (type|interface).*useLocale" -A 5
Length of output: 95
Script:
#!/bin/bash
# Let's try different approaches to find useLocale related code
# 1. Search for useLocale definition
rg "export.+function useLocale"
# 2. Search for Locale type definition
rg "export.+(type|interface) Locale"
# 3. Look for files containing useLocale
fd -e ts -e tsx useLocale
# 4. Check the actual usage of formatTextWithLocale
rg "formatTextWithLocale.+Locale"
# 5. Check the implementation of formatTextWithLocale
ast-grep --pattern 'function formatTextWithLocale($_) { $$$ }'
Length of output: 559
Script:
#!/bin/bash
# Let's check the implementation of useLocale to see what it returns
cat libs/localization/src/lib/useLocale.tsx
# Also check the actual implementation where formatTextWithLocale is defined
rg -g '!*.test.*' -g '!*.spec.*' "function formatTextWithLocale" -A 5
Length of output: 1322
Script:
#!/bin/bash
# Let's find where formatTextWithLocale is defined and its type signature
rg "formatTextWithLocale" -B 2 -A 2
# Also check for any type definitions related to the locale in the application context
rg "type.*Application" -A 5
Length of output: 326111
Script:
#!/bin/bash
# Let's check the implementation of formatTextWithLocale and its type signature
cat libs/application/core/src/lib/formUtils.ts
Length of output: 9330
libs/application/templates/health-insurance/src/fields/TextWithTooltip/TextWithTooltip.tsx (2)
1-1: LGTM: Import statements follow best practices
The imports are well-structured:
- Named imports support tree-shaking
- Types are properly imported from shared packages
Also applies to: 6-6
19-19: 🛠️ Refactor suggestion
Verify the type casting of locale
While the implementation looks correct, the type casting of locale as Locale suggests we might need to strengthen the type safety here.
Let's check how the locale type is handled across the codebase:
Consider updating the useLocale hook typing to return the correct Locale type instead of using type casting. Example:
// In the useLocale hook type definition
interface UseLocaleReturn {
formatMessage: MessageFormatter
lang: Locale // Instead of string
}Also applies to: 24-27
libs/application/templates/funding-government-projects/src/fields/FieldTitle/FieldTitle.tsx (3)
2-2: LGTM: Import statements are well-organized
The imports follow good practices for tree-shaking by importing specific entities rather than entire modules.
Also applies to: 6-6
18-18: LGTM: Clear and consistent variable naming
The renaming from lang to locale improves code clarity and maintains consistency with the PR's localization objectives.
26-31: Verify consistent usage of formatTextWithLocale
Let's ensure this change is consistently applied across similar components.
✅ Verification successful
Based on the search results, I can now provide a definitive response:
Consistent usage of formatTextWithLocale is verified
The change from formatText to formatTextWithLocale for field.title appears to be part of a broader migration pattern. The search results show that formatTextWithLocale is already being used consistently for field titles across multiple components in the codebase, including:
- LinkFormField
- TextFormField
- StaticTableFormField
- SelectFormField
- RadioFormField
- PhoneFormField
- AccordionFormField
- And many others
The change maintains consistency with this established pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for remaining formatText usage in field titles
rg "formatText\(\s*field\.title" -A 2
# Search for components using similar pattern
ast-grep --pattern 'formatText($$$field.title$$$)'
Length of output: 92
Script:
#!/bin/bash
# Search for any remaining formatText usage
rg "formatText\(" -A 2
# Search for formatTextWithLocale usage
rg "formatTextWithLocale\(" -A 2
# Search for field.title usage to ensure we catch all patterns
rg "field\.title" -A 2
Length of output: 177149
libs/application/ui-fields/src/lib/ExpandableDescriptionFormField/ExpandableDescriptionFormField.tsx (2)
1-1: LGTM: Import changes follow best practices
The imports are properly structured for effective tree-shaking and TypeScript type definitions are correctly sourced from shared packages.
Also applies to: 10-10
25-30: LGTM: Proper implementation of localized text formatting
The change correctly implements locale-aware text formatting, supporting the PR's objective of multilingual support.
Let's verify the type compatibility between useLocale and formatTextWithLocale:
libs/application/ui-fields/src/lib/SubmitFormField/SubmitFormField.tsx (3)
3-3: LGTM: Import statement follows best practices
The import statement correctly imports both formatting functions, enabling tree-shaking by specifically importing only the needed functions.
Line range hint 20-49: Verify the consistency of locale handling across field components
The PR objective mentions adding locale support for select, radio, and checkbox fields. We should verify that this component's locale handling is consistent with other field components.
✅ Verification successful
Let me gather more information about the locale handling patterns.
Locale handling is consistent across field components
Based on the verification results, the locale handling in SubmitFormField is consistent with other field components:
- All field components use the same pattern of extracting locale from useLocale hook:
const { formatMessage, lang: locale } = useLocale()- They consistently use the same text formatting utilities:
formatTextWithLocalefor main field labels/titlesformatTextfor descriptions and option labels
- The implementation in SelectFormField, RadioFormField, and CheckboxFormField follows the same pattern as SubmitFormField, using both locale-aware formatting functions with the same parameter structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check locale handling in other field components
echo "Checking locale handling patterns across field components..."
rg -l "formatTextWithLocale.*locale.*formatMessage" --type typescript
echo "Checking useLocale hook usage..."
ast-grep --pattern 'const { $$$, lang: locale } = useLocale()'
Length of output: 4650
Script:
#!/bin/bash
# Check formatTextWithLocale usage in field components
echo "Checking formatTextWithLocale usage patterns..."
rg "formatTextWithLocale\(" --type typescript -A 2
# Check other text formatting patterns
echo -e "\nChecking other text formatting patterns..."
rg "formatText\(" --type typescript -A 2
Length of output: 371
Script:
#!/bin/bash
# Check formatTextWithLocale usage patterns
echo "Checking formatTextWithLocale usage patterns..."
rg "formatTextWithLocale" -A 2
# Check other text formatting patterns
echo -e "\nChecking other text formatting patterns..."
rg "formatText\(" -A 2
# Check specific field components
echo -e "\nChecking specific field components..."
rg "formatText|formatTextWithLocale" libs/application/ui-fields/src/lib/SelectFormField/SelectFormField.tsx libs/application/ui-fields/src/lib/RadioFormField/RadioFormField.tsx libs/application/ui-fields/src/lib/CheckboxFormField/CheckboxFormField.tsx
Length of output: 180792
20-20: Verify the type safety of the locale casting
While the change from lang to locale alias is fine, we should ensure type safety when casting to Locale type later in the code.
✅ Verification successful
Let me gather more information about the type safety of the locale usage.
Let me try one more search to find the actual useLocale implementation in the web application context since the previous search only found the native app implementation.
Let me do one final check to see how the locale is being used in the SubmitFormField component.
Type safety of locale casting is properly maintained
The locale casting is type-safe because:
- The
Localetype is properly imported from '@island.is/shared/types' - The type is defined as
type Locale = 'is' | 'en' - The casting is used in a controlled context with
formatTextWithLocale
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if useLocale hook's return type matches the Locale type
ast-grep --pattern 'export const useLocale = () => {
$$$
return {
$$$
lang: $_,
$$$
}
}'
Length of output: 4650
Script:
#!/bin/bash
# Check the implementation of useLocale hook and its return type
ast-grep --pattern 'export const useLocale = () => {
$$$
}'
# Check how locale is used in SubmitFormField.tsx
rg -A 5 "locale =" "libs/application/ui-fields/src/lib/SubmitFormField/SubmitFormField.tsx"
Length of output: 461
Script:
#!/bin/bash
# Search for useLocale implementation in the web context
rg -g '!apps/native/**' -A 10 "export const useLocale = "
# Also check for any type definitions related to locale
rg -g '*.ts' -g '*.tsx' -g '!apps/native/**' "type Locale = "
Length of output: 192
Script:
#!/bin/bash
# Check the full content of SubmitFormField to see locale usage
rg -g 'SubmitFormField.tsx' "locale" -B 2 -A 5
# Check if there are any type imports for Locale in this file
rg -g 'SubmitFormField.tsx' "import.*Locale"
Length of output: 2142
libs/application/ui-fields/src/lib/LinkFormField/LinkFormField.tsx (1)
2-2: LGTM: Import changes follow best practices
The import statement correctly destructures specific functions, enabling effective tree-shaking.
libs/application/ui-shell/src/hooks/useApplicationTitle.ts (2)
3-3: LGTM: Import changes align with localization requirements
The changes properly utilize shared types and named imports, maintaining good tree-shaking capabilities.
Also applies to: 11-11
Line range hint 1-54: Verify type definitions in useLocale hook
The implementation looks good overall, but let's verify the type definitions in the useLocale hook to ensure we can remove the type assertions safely.
libs/application/ui-fields/src/lib/AccordionFormField/AccordionFormField.tsx (2)
8-8: LGTM: Import changes maintain tree-shaking capability
The specific import of formatTextWithLocale alongside formatText follows good practices for tree-shaking optimization.
39-44: 🛠️ Refactor suggestion
Verify locale type assertion and consider type guard
The type assertion locale as Locale might be unsafe. Consider adding a type guard or validation:
Consider adding a type guard:
function isLocale(value: string): value is Locale {
return ['is', 'en'].includes(value) // adjust based on supported locales
}
// Usage
if (!isLocale(locale)) {
console.warn(`Unsupported locale: ${locale}`)
}✅ Verification successful
Let me analyze the findings from the shell scripts:
-
The
Localetype is defined in two places:libs/shared/types/src/lib/locale.ts:type Locale = 'is' | 'en'apps/native/app/src/stores/preferences-store.ts:type Locale = 'en-US' | 'is-IS' | 'en-IS' | 'is-US'
-
There's already a type guard implementation in
libs/localization/src/lib/LocaleContext.tsx:
return supportedLocales.includes(value as Locale)Let me verify which Locale type is being used in the context of AccordionFormField:
Type assertion is safe and consistent with codebase patterns
The type assertion locale as Locale in AccordionFormField is safe because:
- It uses the canonical
Localetype from@island.is/shared/typeswhich defines only'is' | 'en'values - The codebase already has a type guard in
LocaleContext.tsxthat validates locale values - This pattern is consistently used across 100+ files in the codebase
- The component uses
useLocale()hook from the localization module which provides validated locale values
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if Locale type is properly defined and used consistently
ast-grep --pattern 'type Locale = $_'
rg -A 2 'as Locale'
Length of output: 32809
Script:
#!/bin/bash
# Check imports in AccordionFormField
rg -p "import.*Locale" libs/application/ui-fields/src/lib/AccordionFormField/AccordionFormField.tsx
Length of output: 288
libs/application/ui-fields/src/lib/CompanySearchFormField/CompanySearchFormField.tsx (3)
6-10: LGTM: Import changes follow best practices
The imports are well-organized and tree-shaking friendly with specific named imports.
Line range hint 1-83: Implementation meets coding guidelines
The component successfully:
- Uses TypeScript for type safety
- Remains reusable across different NextJS applications
- Follows proper bundling practices with specific imports
35-35: 🛠️ Refactor suggestion
Consider improving type safety of locale extraction
While the current implementation works, we could enhance type safety and readability.
- const { formatMessage, lang: locale } = useLocale()
+ const { formatMessage, lang } = useLocale()
+ const locale = lang as LocaleLikely invalid or redundant comment.
libs/application/ui-fields/src/lib/DescriptionFormField/DescriptionFormField.tsx (3)
3-3: LGTM: Imports follow best practices
The imports are well-structured with named exports, supporting effective tree-shaking. The explicit import of the Locale type enhances type safety.
Also applies to: 8-8
17-17: LGTM: Clear variable naming
The destructuring with renaming improves code clarity by making the variable name match its usage context.
Line range hint 9-16: Verify component usage across applications
The component is well-structured for reuse. Let's verify its usage pattern across different applications to ensure consistent implementation.
✅ Verification successful
Component usage pattern is consistent across applications
The verification shows that the DescriptionFormField component is being used consistently across the codebase:
- It's properly exported from
libs/application/ui-fieldsand used in multiple applications - Usage patterns are consistent, with all implementations providing the required props (application, field, showFieldName)
- The component is primarily used in review and form sections of the directorate-of-immigration citizenship application
- Documentation (stories.mdx) provides clear usage guidelines that match the actual implementations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for DescriptionFormField usage patterns
ast-grep --pattern 'import { DescriptionFormField } from "@island.is/application/ui-fields"'
Length of output: 94
Script:
#!/bin/bash
# Search for DescriptionFormField usage with ripgrep
rg "DescriptionFormField" -A 2 -B 2
# Also search for potential different import paths
rg 'from.*ui-fields.*DescriptionFormField' -A 1 -B 1
# Search for files containing DescriptionFormField in their name
fd "DescriptionFormField"
Length of output: 33651
libs/application/templates/institution-collaboration/src/fields/SecondaryContact/SecondaryContact.tsx (3)
8-8: LGTM: Import changes are well-structured
The addition of formatTextWithLocale and Locale type imports properly supports the new locale-aware text formatting functionality while maintaining strong typing.
Also applies to: 12-12
19-19: LGTM: Improved variable naming for locale
The renaming from lang to locale improves code clarity and maintains consistency with the type system.
Line range hint 1-77: LGTM: Component structure follows best practices
The component adheres to all required guidelines for library code:
- Properly typed props using TypeScript
- Reusable design suitable for different NextJS apps
- Clean and maintainable structure
libs/application/ui-fields/src/lib/AlertMessageFormField/AlertMessageFormField.tsx (4)
1-1: LGTM: Import changes are appropriate
The addition of formatTextWithLocale while maintaining formatText is correct as both functions are used in the component.
7-7: LGTM: Type import follows best practices
The Locale type from shared types ensures consistent typing across the application.
17-17: LGTM: Improved variable naming
Renaming lang to locale improves code clarity and aligns with its usage in formatTextWithLocale.
25-30: Consider improving type safety
While the changes correctly implement locale support, consider defining the locale type at the hook level rather than using a type assertion here.
Let's verify the consistent usage of formatTextWithLocale across the codebase:
✅ Verification successful
Type assertion pattern is consistent across the codebase
The search results show that all usages of formatTextWithLocale follow the same pattern of type assertion locale as Locale across the entire codebase. This consistent pattern suggests it's an intentional design choice rather than a one-off implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of formatTextWithLocale
# Expected: All calls should include the locale parameter
# Search for all usages of formatTextWithLocale
rg "formatTextWithLocale\(" -A 5
Length of output: 23396
libs/application/ui-fields/src/lib/PhoneFormField/PhoneFormField.tsx (3)
3-3: LGTM: Import changes follow best practices
The imports are well-organized and support effective tree-shaking. The types are correctly imported from the shared package.
Also applies to: 12-12
40-40: LGTM: Improved variable naming for locale
The renaming of lang to locale improves code clarity and aligns with the localization enhancement objective.
Line range hint 1-89: Verify integration with other form fields
Since this component is part of the shared UI library, we should ensure consistent locale handling across other form field components.
✅ Verification successful
Locale handling is consistent across form field components
The verification shows that the locale handling pattern using locale as Locale with formatTextWithLocale is consistently implemented across all form field components in the shared UI library, including:
- AccordionFormField
- AlertMessageFormField
- AsyncSelectFormField
- BankAccountFormField
- CheckboxFormField
- DescriptionFormField
- DividerFormField
- ExpandableDescriptionFormField
- ImageFormField
- LinkFormField
- PhoneFormField
- RadioFormField
- SelectFormField
- StaticTableFormField
- SubmitFormField
- TableRepeaterFormField
- TextFormField
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for consistent locale handling in other form fields
echo "Checking for consistent locale handling patterns..."
rg -l "formatTextWithLocale" libs/application/ui-fields/src/lib/ | while read -r file; do
echo "=== $file ==="
rg "locale as Locale" "$file" -C 2
done
Length of output: 8662
libs/application/templates/operating-license/src/fields/PropertyOverviewRepeater/index.tsx (2)
7-11: LGTM! Well-organized imports following best practices.
The imports are properly organized and follow tree-shaking practices by importing specific functions and types.
32-37: Verify the impact of the new locale parameter.
The function call has been updated correctly to include the locale parameter. However, we should verify that this change is consistent with other usages of formatTextWithLocale.
✅ Verification successful
All usages of formatTextWithLocale consistently include the locale parameter
The verification shows that all 35 occurrences of formatTextWithLocale across the codebase follow the same pattern, consistently including the locale parameter as the third argument. The change in the review file is aligned with the codebase-wide usage pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of formatTextWithLocale across the codebase
# Expect: All calls should include the locale parameter
# Search for all usages of formatTextWithLocale
ast-grep --pattern 'formatTextWithLocale($$$)'
Length of output: 23487
libs/application/ui-fields/src/lib/CheckboxFormField/CheckboxFormField.tsx (4)
8-8: LGTM: Import additions are well-structured
The new imports for formatTextWithLocale and Locale type are correctly placed and follow TypeScript best practices.
Also applies to: 19-19
45-45: LGTM: Clean hook usage with clear aliasing
The extraction of locale from useLocale hook using lang: locale aliasing is clean and follows React best practices.
48-49: Verify type safety of locale parameter
While the changes look good, let's verify that the locale parameter matches the expected type in buildFieldOptions.
✅ Verification successful
Type safety of locale parameter is verified
The buildFieldOptions function's type signature shows that the locale parameter is correctly typed as part of the MaybeWithApplicationAndFieldAndLocale type, which is imported from '@island.is/application/types'. The function properly handles both direct option arrays and function-based options that receive the locale parameter.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the type definition of buildFieldOptions
ast-grep --pattern 'buildFieldOptions($params) {
$$$
}'
Length of output: 59
Script:
#!/bin/bash
# Let's try to find the buildFieldOptions function with a more flexible pattern
ast-grep --pattern 'buildFieldOptions'
# Also search for its type definition or export
rg "buildFieldOptions" -A 3
Length of output: 3672
Script:
#!/bin/bash
# Let's check the type definition of buildFieldOptions in fieldBuilders.ts
cat libs/application/core/src/lib/fieldBuilders.ts
Length of output: 21099
56-61: Consider removing explicit type assertion
The type assertion locale as Locale might be unnecessary if the locale value from useLocale hook is already properly typed.
✅ Verification successful
Let me check the type of lang from the LocaleContext to ensure we have all the information needed.
Let me check the LocaleContextType definition to understand the type of lang.
Let me try a different pattern to find the type definition.
Type assertion is necessary, keep the current implementation
The type assertion locale as Locale is actually necessary because there are two different Locale types in the codebase:
libs/shared/types/src/lib/locale.tsdefines it as'is' | 'en'apps/native/app/src/stores/preferences-store.tsdefines it as'en-US' | 'is-IS' | 'en-IS' | 'is-US'
The formatTextWithLocale function likely expects the first type ('is' | 'en'), while the useLocale hook's lang property might be using a different type, making the type assertion necessary for type safety.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the return type of useLocale hook
ast-grep --pattern 'export function useLocale() {
$$$
}'
Length of output: 2311
Script:
#!/bin/bash
# Check LocaleContext type definition
ast-grep --pattern 'export const LocaleContext = createContext<$_>($_)'
Length of output: 663
Script:
#!/bin/bash
# Check LocaleContextType definition
ast-grep --pattern 'type LocaleContextType = {
$$$
}'
Length of output: 57
Script:
#!/bin/bash
# Check interface or type definition for LocaleContextType
ast-grep --pattern 'interface LocaleContextType {
$$$
}'
# Also check for any Locale type definitions
ast-grep --pattern 'type Locale = $_'
Length of output: 874
libs/application/ui-fields/src/lib/TextFormField/TextFormField.tsx (2)
49-49: LGTM: Proper destructuring of useLocale hook
The destructuring of lang as locale from useLocale hook is correct and maintains consistency with the localization pattern.
65-70: Verify the impact of locale changes on formatTextWithLocale usage
The function is used consistently for both placeholder and label formatting, but we should verify its implementation.
Also applies to: 73-78
✅ Verification successful
Let me analyze the usage of formatTextWithLocale further by checking its implementation and tests.
Let me check the implementation of the locale handling and message formatting utilities to understand the complete context.
Locale handling in formatTextWithLocale is implemented correctly and safely
Based on the implementation analysis:
- The function properly handles different text formats (string, array, object) with type safety
- It correctly processes locale-specific messages through the MessageFormatter
- The implementation includes proper null/undefined checks
- The function is used consistently across form fields for both placeholders and labels
- Type definitions ensure type safety for all text transformations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of formatTextWithLocale to ensure consistent locale handling
rg "formatTextWithLocale" -A 3 -B 1
# Look for potential test coverage
rg "formatTextWithLocale.*test"
Length of output: 39391
Script:
#!/bin/bash
# Search for the implementation of formatTextWithLocale
ast-grep --pattern 'export const formatTextWithLocale = $_'
# Search for test files related to formatTextWithLocale
fd -e test.ts -e test.tsx -e spec.ts -e spec.tsx | xargs rg "formatTextWithLocale"
Length of output: 3204
Script:
#!/bin/bash
# Look for the handleMessageFormatting implementation
ast-grep --pattern 'const handleMessageFormatting = $_'
# Search for the types used in formatTextWithLocale
rg "type (FormTextArray|FormText|FormTextWithLocale|StaticText|StaticTextObject)" -A 3
# Look for any locale-related tests
fd "locale" -e test.ts -e test.tsx -e spec.ts -e spec.tsx | xargs rg "describe|it\("
Length of output: 3225
libs/application/ui-fields/src/lib/BankAccountFormField/BankAccountFormField.tsx (2)
4-4: LGTM: Import changes align with localization requirements
The addition of formatTextWithLocale and Locale type imports properly supports the localization enhancement while maintaining type safety.
Also applies to: 10-10
17-17: LGTM: Consistent locale extraction from useLocale hook
The destructuring assignment properly extracts and renames the language code to maintain consistency with the locale parameter usage throughout the codebase.
libs/application/ui-fields/src/lib/AsyncSelectFormField/AsyncSelectFormField.tsx (3)
3-3: LGTM: Import changes follow best practices
The addition of formatTextWithLocale import maintains good tree-shaking practices by importing from the same module.
15-15: LGTM: Type import enhances type safety
The addition of the Locale type import follows TypeScript best practices and ensures proper type checking.
Line range hint 63-94: Consider updating remaining formatText calls
There are several other instances of formatText in the file that might benefit from locale support:
- Description formatting
- Options label and tooltip formatting
- Placeholder formatting
Consider whether these should also use formatTextWithLocale for consistency.
libs/application/templates/grindavik-housing-buyout/src/fields/AdditionalOwnersOverview/index.tsx (2)
14-14: LGTM! Import changes follow best practices
The import changes maintain good TypeScript practices with explicit type imports and utilize shared packages effectively.
Also applies to: 21-21
40-40: LGTM! Clear and consistent locale handling
The destructuring of useLocale hook with renaming lang to locale improves code clarity and maintains consistency with the localization enhancement objective.
libs/application/ui-fields/src/lib/StaticTableFormField/StaticTableFormField.tsx (3)
5-5: LGTM: Import changes follow best practices
The addition of formatTextWithLocale maintains good tree-shaking practices by importing specific functions.
Line range hint 10-13: LGTM: Component follows library guidelines
The component properly:
- Uses TypeScript for props definition
- Extends base types for reusability
- Exports necessary types for consuming applications
35-40: Verify intentional difference in localization handling
The title now uses formatTextWithLocale while other text elements (description, cells, summary) continue to use formatText. Please confirm if this difference is intentional.
✅ Verification successful
Based on the search results, I can see that formatTextWithLocale is consistently used for titles and headings across the codebase, while formatText is used for other text elements like descriptions, labels, messages, etc. This pattern appears to be intentional and follows a consistent design pattern throughout the application.
The difference in localization handling is intentional and follows the codebase pattern
The codebase shows a clear separation where:
formatTextWithLocaleis used for main titles and headings that require explicit locale handlingformatTextis used for supporting text elements where default locale behavior is sufficient
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other components' usage patterns of formatText vs formatTextWithLocale
echo "Checking formatTextWithLocale usage patterns:"
rg "formatTextWithLocale" -A 2 -B 2
echo "\nChecking formatText usage patterns:"
rg "formatText\([^W]" -A 2 -B 2
Length of output: 143854
libs/application/templates/announcement-of-death/src/fields/FilesRecipientCard/index.tsx (3)
4-4: LGTM: Import changes follow TypeScript best practices.
The additions of formatTextWithLocale and Locale type imports maintain strong typing and reusability across the application.
Also applies to: 14-14
70-75: LGTM: Proper implementation of locale-aware text formatting.
The update to use formatTextWithLocale aligns with the PR's objective of enhancing localization support. The implementation is consistent and includes all necessary parameters.
28-28: 🛠️ Refactor suggestion
Consider avoiding type assertion by properly typing the locale.
The type assertion locale as Locale later in the code could be avoided by properly typing the useLocale hook return value.
Let's check if the useLocale hook's return type can be improved:
libs/application/ui-shell/src/components/FormStepper.tsx (4)
13-14: LGTM: Clean import additions
The new imports are well-structured and follow TypeScript best practices while maintaining good tree-shaking potential.
34-34: LGTM: Clear variable naming
The destructuring with renaming improves code clarity by making the locale's purpose more explicit.
100-104: Type safety improvement needed here as well
Same type assertion pattern as noted in the parseSubsections function.
Line range hint 1-132: Verify the impact on dependent components
Let's ensure that all components using FormStepper are compatible with the new locale-aware text formatting.
✅ Verification successful
Let me gather more information about the FormStepper usage to make a more informed decision.
Let me check the usage of formatTextWithLocale to ensure we have a complete picture.
No compatibility issues with locale-aware text formatting
The changes in FormStepper are safe as:
- The component is already using
formatTextWithLocalefrom@island.is/application/corecorrectly - All components using FormStepper are importing it from the UI shell package which handles the locale formatting
- The locale handling is consistent across the codebase with proper imports and usage patterns
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for FormStepper usage patterns
rg -l "FormStepper" | xargs rg -l "formatText[^W]" || echo "No potential incompatibilities found"
Length of output: 133
Script:
#!/bin/bash
# Search for all FormStepper usages with context
rg -A 5 "FormStepper"
# Also check if there are any direct imports of FormStepper
rg "import.*FormStepper"
Length of output: 64782
Script:
#!/bin/bash
# Search for formatTextWithLocale usage
rg -A 3 "formatTextWithLocale"
# Also check for any imports of formatTextWithLocale
rg "import.*formatTextWithLocale"
Length of output: 35165
libs/application/ui-fields/src/lib/DateFormField/DateFormField.tsx (1)
3-7: LGTM! Import changes align with localization requirements.
The imports are properly structured and follow TypeScript best practices. Both formatText and formatTextWithLocale are utilized within the component for different formatting needs.
Also applies to: 21-21
libs/application/templates/data-protection-complaint/src/fields/CommissionFieldRepeater/CommissionFieldRepeater.tsx (1)
3-7: LGTM: Import changes maintain tree-shaking and type safety
The imports are well-organized and maintain effective tree-shaking by importing specific functions and types.
Also applies to: 20-20
libs/application/ui-fields/src/lib/ImageFormField/ImageFormField.tsx (3)
5-5: LGTM: Import change aligns with localization enhancement
The replacement of formatText with formatTextWithLocale from the core module maintains good reusability practices.
9-9: LGTM: Type import follows best practices
The Locale type import from shared types promotes type reusability across the application.
54-54: LGTM: Clear variable naming in hook usage
The destructuring rename from lang to locale improves code clarity and maintains consistency with the localization context.
libs/application/types/src/lib/Form.ts (3)
12-12: LGTM: Clean import addition
The import of the Locale type from the shared package follows best practices and maintains consistency with the existing import style.
105-105: Verify the impact of this breaking change
Changing the title property type from FormText to FormTextWithLocale is a breaking change that affects all implementations of FormItem.
Let's check for potential impacts:
Consider adding a migration guide in the documentation to help developers update their implementations.
36-41: LGTM: Well-structured type definition with proper locale support
The FormTextWithLocale type effectively extends the existing pattern while adding locale support. It maintains type safety and follows TypeScript best practices.
Let's verify the consistent usage of this new type across the codebase:
✅ Verification successful
No issues found with the FormTextWithLocale type implementation
The codebase analysis shows that the FormTextWithLocale type is properly implemented and used consistently. The type is used in form titles and other text fields where locale support is needed, with proper handling of both the application and locale parameters. The implementation in formatTextWithLocale function correctly processes both parameters.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of FormTextWithLocale
# Expect: All implementations should handle both the application and locale parameters
# Search for implementations of functions that match the FormTextWithLocale type
ast-grep --pattern 'application: Application, locale: Locale) => $$$'
Length of output: 136
Script:
#!/bin/bash
# Let's try a broader search to find usages of FormTextWithLocale
rg "FormTextWithLocale" -A 3 -B 3
# Also search for potential implementations using a more flexible pattern
ast-grep --pattern '(application: Application, locale: $_) => $$$'
# Search for files that might be using this type
fd -e ts -e tsx -x grep -l "FormTextWithLocale" {}
Length of output: 3118
Script:
#!/bin/bash
# Let's examine the formatTextWithLocale function implementation
rg -A 10 "formatTextWithLocale = " libs/application/core/src/lib/formUtils.ts
# Check for any existing implementations that might need updates
rg "application: Application\) =>" -A 2
# Look for any title implementations in form items
ast-grep --pattern 'title: $_'
Length of output: 257049
libs/application/templates/operating-license/src/fields/PropertyRepeater/index.tsx (3)
24-27: LGTM! Import changes follow best practices.
The imports are properly structured for tree-shaking and maintain type safety.
29-29: LGTM! Proper type import from shared package.
Using shared types ensures consistency across the application.
49-54: Verify locale type compatibility.
The explicit casting of locale to Locale type suggests a potential type mismatch. Let's verify the type compatibility between useLocale's output and the expected Locale type.
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (3)
1-5: LGTM: Import changes follow best practices.
The imports are well-structured, supporting tree-shaking and maintaining proper TypeScript typing.
Also applies to: 27-27
62-62: LGTM: Consistent locale handling.
The renaming of lang to locale aligns with the PR's objective and maintains type safety.
120-125: 🛠️ Refactor suggestion
Consider improving type safety and consistency.
While the change to use formatTextWithLocale is good, there are two areas for improvement:
- The type assertion
locale as Localesuggests a potential type mismatch. Consider handling this at the hook level. - The component still uses
formatTextfor other text formatting (e.g., informatTableValue). Consider usingformatTextWithLocaleconsistently throughout the component for better maintainability.
Let's verify the type definition of the useLocale hook:
Consider these improvements:
- const { formatMessage, lang: locale } = useLocale()
+ const { formatMessage, lang } = useLocale()
+ const locale: Locale = lang
// ... later in the code ...
- {formatTextWithLocale(
- title,
- application,
- locale as Locale,
- formatMessage,
- )}
+ {formatTextWithLocale(
+ title,
+ application,
+ locale,
+ formatMessage,
+ )}libs/application/core/src/lib/formUtils.ts (2)
27-27: LGTM! Import changes follow best practices.
The new imports are properly structured using named imports from shared packages, which supports effective tree-shaking and promotes reusability across applications.
Also applies to: 29-29
282-289: LGTM! Type-safe implementation of locale support.
The function signature properly extends the existing functionality with locale support while maintaining type safety through generics and proper TypeScript types.
libs/application/ui-shell/src/components/Screen.tsx (1)
13-13: LGTM: Import changes follow best practices
The imports are well-structured, using named imports for better tree-shaking and importing types from shared packages.
Also applies to: 58-58
libs/application/types/src/lib/Fields.ts (4)
11-17: LGTM! Import changes are well-organized.
The addition of FormTextWithLocale import is necessary for the localization enhancements.
35-37: Well-designed type definition for locale support!
The new type MaybeWithApplicationAndFieldAndLocale<T> follows TypeScript best practices by:
- Maintaining consistency with existing patterns
- Supporting both static values and dynamic generation with locale
- Enabling reuse across different field types
286-286: Consistent type updates for option-based fields!
The options property type has been updated consistently across CheckboxField, RadioField, and SelectField to support localization using MaybeWithApplicationAndFieldAndLocale<Option[]>.
#!/bin/bash
# Verify consistent implementation of options with locale
ast-grep --pattern 'options: MaybeWithApplicationAndFieldAndLocale<$_>'Also applies to: 323-323, 336-336
198-198: Verify the BaseField title property type change impact.
The change from FormText to FormTextWithLocale for the title property affects all field types. Please ensure that all implementations handle the new locale-aware title type correctly.
libs/application/templates/health-insurance/src/fields/TextWithTooltip/TextWithTooltip.tsx
Show resolved
Hide resolved
...lication/ui-fields/src/lib/ExpandableDescriptionFormField/ExpandableDescriptionFormField.tsx
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/CompanySearchFormField/CompanySearchFormField.tsx
Show resolved
Hide resolved
...tes/data-protection-complaint/src/fields/CommissionFieldRepeater/CommissionFieldRepeater.tsx
Show resolved
Hide resolved
|
Affected services are: api,application-system-api,air-discount-scheme-web,skilavottord-web,web,application-system-form,island-ui-storybook,portals-admin,service-portal,system-e2e, Deployed services: application-system-api,application-system-form,service-portal,portals-admin,api,web,skilavottord-web,island-ui-storybook,air-discount-scheme-web,application-system-api-worker,consultation-portal. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- libs/application/ui-shell/src/hooks/useApplicationTitle.spec.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/application/ui-shell/src/hooks/useApplicationTitle.spec.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."
🔇 Additional comments (2)
libs/application/ui-shell/src/hooks/useApplicationTitle.spec.ts (2)
16-16: LGTM: Clean setup for locale support
The import and locale setup are well-structured and follow TypeScript best practices.
Also applies to: 20-20
84-88: LGTM: Consistent test updates
The test cases have been properly updated to accommodate the new function signature with locale parameter.
Also applies to: 95-99, 106-110
...
Attach a link to issue if relevant
What
Add locale to arguments passed to options functions for select, radio and checkbox fields
Why
To enable different options for different languages
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
careerIndustryfield based on the application's locale.formatTextWithLocalefor locale-aware text formatting across various components.Bug Fixes
Refactor
formatTextwithformatTextWithLocalein multiple components for better localization support.