-
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(application-system): Making title an optional property on form fields #17634
Conversation
WalkthroughThis pull request introduces a comprehensive update to how Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Note: This comprehensive update ensures consistent and robust handling of title properties across the application, preventing potential undefined value errors and improving overall type safety. ✨ Finishing Touches
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 ✅ 10 Total Test Services: 0 Failed, 10 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (2) |
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: 0
🧹 Nitpick comments (15)
libs/application/templates/data-protection-complaint/src/fields/FieldLabel.tsx (1)
12-12
: Consider using nullish coalescing for title defaultWhile providing a default empty string is good, using nullish coalescing would be more explicit about handling both undefined and null cases.
- const { title = '' } = field + const { title } = field + const finalTitle = title ?? ''libs/application/ui-fields/src/lib/CompanySearchFormField/CompanySearchFormField.tsx (1)
29-29
: Consider updating the Props interface to reflect optional title.While adding a default value works, the TypeScript interface should be updated to properly type
title
as optional in theCompanySearchField
type.This would improve type safety and make the optionality more explicit in the type system:
// In the CompanySearchField interface interface CompanySearchField { - title: string; + title?: string; }libs/application/ui-fields/src/lib/AlertMessageFormField/AlertMessageFormField.tsx (1)
26-26
: Consider using consistent default value pattern.While the nullish coalescing operator works here, consider using the same destructuring pattern with default value as other components for consistency:
- field.title ?? '', + title = '',Also, consider updating the
AlertMessageField
interface to maketitle
optional for better type safety.libs/application/ui-fields/src/lib/PhoneFormField/PhoneFormField.tsx (2)
32-32
: LGTM! Consider TypeScript improvements.The implementation is clean and consistent with other components. Consider updating the
PhoneField
interface to maketitle
optional for better type safety.// In the PhoneField interface interface PhoneField { - title: string; + title?: string; }
Line range hint
1-1
: Consider a unified approach to optional titles.While the changes successfully make titles optional, consider:
- Updating all field interfaces to mark
title
as optional using TypeScript- Using consistent patterns for default values (either destructuring with default or nullish coalescing)
- Adding JSDoc comments to document the optionality in the base interfaces
This would improve type safety and maintainability across the codebase.
libs/application/templates/grindavik-housing-buyout/src/fields/AdditionalOwnersOverview/index.tsx (1)
47-47
: Consider using consistent pattern for title defaults.While
??
operator works correctly here, consider using the sametitle = ''
destructuring pattern used in other components for consistency across the codebase.- field.title ?? '', + title,And in the destructuring:
+ const { title = '' } = field;
libs/application/ui-fields/src/lib/RadioFormField/RadioFormField.tsx (1)
Line range hint
108-116
: Consider memoizing parsed HTML content.The HtmlParser calls within the options map could be optimized using useMemo to prevent unnecessary re-parsing on each render.
+ const parsedSubLabel = useMemo( + () => subLabel && HtmlParser(formatText(subLabel, application, formatMessage)), + [subLabel, application, formatMessage] + ) + const parsedTooltip = useMemo( + () => tooltip && HtmlParser(formatText(tooltip, application, formatMessage)), + [tooltip, application, formatMessage] + ) options={finalOptions.map(({ label, subLabel, tooltip, ...o }) => ({ ...o, label: ( <Markdown> {formatText(label, application, formatMessage)} </Markdown> ), - subLabel: subLabel && HtmlParser(formatText(subLabel, application, formatMessage)), + subLabel: parsedSubLabel, ...(tooltip && { - tooltip: HtmlParser(formatText(tooltip, application, formatMessage)), + tooltip: parsedTooltip, }), }))}libs/application/templates/data-protection-complaint/src/fields/CommissionFieldRepeater/CommissionFieldRepeater.tsx (1)
Line range hint
18-29
: Consider making the component more reusable.The component is currently specific to the data-protection-complaint template but could be made more generic for reuse across different templates by:
- Making the field structure configurable
- Extracting template-specific labels to props
+ interface FieldConfig { + name: string; + label: string; + format?: string; + } + + interface CommissionFieldRepeaterProps extends FieldBaseProps { + fields: FieldConfig[]; + addButtonLabel: string; + } - type PersonField = { - name: string; - nationalId: string; - }libs/application/ui-fields/src/lib/DateFormField/DateFormField.tsx (1)
Line range hint
52-82
: Consider extracting date computation functions.The date computation functions could be extracted to a separate utility file for better code organization and reusability.
+ // dateUtils.ts + export const computeMinDate = ( + maybeMinDate: MaybeWithApplicationAndField<Date>, + memoApplication: Application, + memoField: DateField, + ) => { + if (typeof maybeMinDate === 'function') { + return maybeMinDate(memoApplication, memoField) + } + return maybeMinDate + } + + export const computeMaxDate = /* ... */ + + export const computeExcludeDates = /* ... */libs/application/ui-fields/src/lib/AsyncSelectFormField/AsyncSelectFormField.tsx (1)
33-33
: Consider improving type safety for the title property.The default empty string for
title
aligns with making it optional. However, to improve type safety, consider updating the field type in the Props interface to reflect this optionality.interface Props extends FieldBaseProps { - field: AsyncSelectField + field: Omit<AsyncSelectField, 'title'> & { title?: string } }libs/application/types/src/lib/Form.ts (1)
86-86
: LGTM! Consider adding JSDoc comments.The changes consistently make the
title
property optional across all interfaces, which aligns with the PR objectives. To improve maintainability, consider adding JSDoc comments to document this intentional optionality.interface Form { + /** Optional title that defaults to an empty string if not provided */ title?: StaticText // ... other properties }
Also applies to: 105-105, 156-156, 165-165
libs/application/ui-shell/src/components/Screen.tsx (1)
352-352
: Consider using destructuring with default value for consistency.While the null coalescing operator works, consider using destructuring with a default value to maintain consistency with other files:
- screen.title ?? '', + title,And at the top of the component:
const { title = '' } = screenlibs/application/core/src/lib/fieldBuilders.ts (1)
598-598
: Add default values for title in remaining builder functions.For consistency, consider adding default empty string values when destructuring
title
in these functions as well.Apply this pattern to the affected functions:
-const { id, url, message, buttonTitle } = data +const { id, url, message, buttonTitle, title = '' } = data -const { id, description, introText, startExpanded } = data +const { id, description, introText, startExpanded, title = '' } = data -const { id, forPaymentLabel, totalLabel, getSelectedChargeItems } = data +const { id, forPaymentLabel, totalLabel, getSelectedChargeItems, title = '' } = dataAlso applies to: 614-614, 660-660
libs/application/ui-shell/src/components/FormStepper.tsx (2)
62-62
: LGTM! Consider improving type safety.The nullish coalescing operator ensures safe handling of undefined titles. However, we could further improve type safety by explicitly typing the title property in the SectionChildren interface.
interface SectionChildren { id: string; title?: MessageDescriptor | string; // ... other properties }
101-101
: LGTM! Consider similar type improvement for section titles.The nullish coalescing operator is consistently applied here as well. For completeness, consider applying the same type improvement to the TSection interface.
interface Section { title?: MessageDescriptor | string; children: Array<SectionChildren>; // ... other properties }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
libs/application/core/src/lib/fieldBuilders.ts
(11 hunks)libs/application/core/src/lib/formBuilders.ts
(2 hunks)libs/application/templates/data-protection-complaint/src/fields/CommissionFieldRepeater/CommissionFieldRepeater.tsx
(1 hunks)libs/application/templates/data-protection-complaint/src/fields/FieldLabel.tsx
(1 hunks)libs/application/templates/funding-government-projects/src/fields/FieldTitle/FieldTitle.tsx
(1 hunks)libs/application/templates/grindavik-housing-buyout/src/fields/AdditionalOwnersOverview/index.tsx
(1 hunks)libs/application/templates/institution-collaboration/src/fields/SecondaryContact/SecondaryContact.tsx
(1 hunks)libs/application/templates/operating-license/src/fields/PropertyOverviewRepeater/index.tsx
(1 hunks)libs/application/templates/operating-license/src/fields/PropertyRepeater/index.tsx
(1 hunks)libs/application/types/src/lib/Fields.ts
(1 hunks)libs/application/types/src/lib/Form.ts
(4 hunks)libs/application/ui-fields/src/lib/AlertMessageFormField/AlertMessageFormField.tsx
(1 hunks)libs/application/ui-fields/src/lib/AsyncSelectFormField/AsyncSelectFormField.tsx
(1 hunks)libs/application/ui-fields/src/lib/CheckboxFormField/CheckboxFormField.tsx
(1 hunks)libs/application/ui-fields/src/lib/CompanySearchFormField/CompanySearchFormField.tsx
(1 hunks)libs/application/ui-fields/src/lib/DateFormField/DateFormField.tsx
(1 hunks)libs/application/ui-fields/src/lib/DescriptionFormField/DescriptionFormField.tsx
(1 hunks)libs/application/ui-fields/src/lib/ExpandableDescriptionFormField/ExpandableDescriptionFormField.tsx
(1 hunks)libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.tsx
(1 hunks)libs/application/ui-fields/src/lib/LinkFormField/LinkFormField.tsx
(1 hunks)libs/application/ui-fields/src/lib/PhoneFormField/PhoneFormField.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)libs/application/ui-fields/src/lib/StaticTableFormField/StaticTableFormField.tsx
(1 hunks)libs/application/ui-fields/src/lib/SubmitFormField/SubmitFormField.tsx
(1 hunks)libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx
(1 hunks)libs/application/ui-fields/src/lib/TextFormField/TextFormField.tsx
(1 hunks)libs/application/ui-fields/src/lib/TitleFormField/TitleFormField.tsx
(1 hunks)libs/application/ui-shell/src/components/FormExternalDataProvider.tsx
(3 hunks)libs/application/ui-shell/src/components/FormStepper.tsx
(2 hunks)libs/application/ui-shell/src/components/Screen.tsx
(1 hunks)libs/application/ui-shell/src/hooks/useApplicationTitle.ts
(1 hunks)libs/application/ui-shell/src/lib/FormShell.tsx
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (33)
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/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/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/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/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/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/ui-fields/src/lib/TitleFormField/TitleFormField.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/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/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-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-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/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/FieldsRepeaterFormField/FieldsRepeaterFormField.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."
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/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/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/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-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-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-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/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/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/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-shell/src/lib/FormShell.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-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."
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-shell/src/components/FormExternalDataProvider.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/core/src/lib/formBuilders.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/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."
🔇 Additional comments (28)
libs/application/ui-fields/src/lib/TitleFormField/TitleFormField.tsx (1)
17-17
: LGTM! Consistent with other form field componentsThe default empty string for title aligns with the PR objective and matches the pattern used in other components.
libs/application/templates/funding-government-projects/src/fields/FieldTitle/FieldTitle.tsx (1)
27-27
: LGTM! Excellent use of nullish coalescingThe nullish coalescing operator is the ideal choice here as it explicitly handles both undefined and null cases.
libs/application/ui-fields/src/lib/ExpandableDescriptionFormField/ExpandableDescriptionFormField.tsx (1)
27-27
: LGTM! Consistent with other componentsThe nullish coalescing operator usage matches the pattern in FieldTitle, maintaining consistency across the codebase.
libs/application/ui-fields/src/lib/SubmitFormField/SubmitFormField.tsx (1)
19-26
: LGTM! Default value for title enhances component robustness.The addition of a default empty string for the title property aligns with making it optional while preventing undefined rendering issues.
Let's verify other usages of this component to ensure they handle the optional title correctly:
✅ Verification successful
Default empty string for title is type-safe and properly handled
The change is verified to be safe because:
- The component is part of a form fields library with proper type definitions
- Empty strings are safely handled by the formatTextWithLocale utility and Text component
- The component is properly encapsulated in the UI library
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for SubmitFormField usages ast-grep --pattern 'SubmitFormField $$$'Length of output: 42
Script:
#!/bin/bash # Search for SubmitFormField imports and usages with ripgrep rg -l "SubmitFormField" --type ts --type tsx # Get the component's type definitions cat libs/application/ui-fields/src/lib/SubmitFormField/SubmitFormField.tsx # Search for JSX usage patterns rg "<SubmitFormField" --type tsx -A 2Length of output: 1785
Script:
#!/bin/bash # Search for SubmitFormField usages with explicit extension patterns rg -l "SubmitFormField" --type-add 'typescript:*.{ts,tsx}' --type typescript # Search for JSX usage patterns rg "<SubmitFormField" --type-add 'typescript:*.{ts,tsx}' --type typescript -A 3Length of output: 314
libs/application/ui-fields/src/lib/LinkFormField/LinkFormField.tsx (1)
53-53
: LGTM! Nullish coalescing operator handles optional title properly.The use of
??
operator is the correct TypeScript pattern for providing a default value for an optional property.libs/application/ui-shell/src/hooks/useApplicationTitle.ts (1)
31-31
: LGTM! Consistent handling of optional titles.The changes properly handle optional titles in both form and section contexts, maintaining consistent behavior throughout the application shell.
Also applies to: 36-36
libs/application/core/src/lib/formBuilders.ts (2)
15-15
: LGTM! Consistent implementation of optional title across form builders.The changes systematically implement optional title handling across all basic form builders using the same pattern.
Also applies to: 19-19, 23-23, 27-27, 31-31
39-39
: LGTM! Data provider builders follow the same optional title pattern.The changes maintain consistency in handling optional titles across data provider builders, aligning with the rest of the form builders.
Also applies to: 53-53
libs/application/templates/institution-collaboration/src/fields/SecondaryContact/SecondaryContact.tsx (1)
20-20
: LGTM! Clean implementation of optional title.The change maintains component functionality while making the title optional, consistent with the PR objectives.
libs/application/ui-fields/src/lib/SelectFormField/SelectFormField.tsx (1)
31-31
: LGTM! Type-safe default value implementation.The empty string default for
title
maintains type safety while making the property effectively optional, aligning with the PR objectives.libs/application/ui-fields/src/lib/CheckboxFormField/CheckboxFormField.tsx (1)
34-34
: Verify showFieldName behavior with empty title.While the empty string default for
title
is correct, verify thatshowFieldName
properly handles the case when title is empty, as it directly affects the Text component rendering.✅ Verification successful
Empty title handling is implemented safely
The implementation correctly handles empty titles when
showFieldName
is true. The Text component will simply render without content, which is a safe behavior. TheformatTextWithLocale
function properly processes empty strings, preventing any potential issues.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for test cases covering showFieldName behavior rg -l "showFieldName.*true" --type ts --type tsxLength of output: 78
Script:
#!/bin/bash # Find showFieldName prop usage in implementation ast-grep --pattern 'showFieldName={$_}' # Search for test files and content rg -t js -t jsx -t ts "showFieldName.*true" # Find CheckboxFormField test files fd "CheckboxFormField.*test" --type fLength of output: 155
Script:
#!/bin/bash # View the component implementation cat libs/application/ui-fields/src/lib/CheckboxFormField/CheckboxFormField.tsx # Broader search for test files find . -type f -name "CheckboxFormField*test*" # Search for showFieldName without file type restrictions rg "showFieldName"Length of output: 8402
libs/application/ui-fields/src/lib/TextFormField/TextFormField.tsx (1)
34-34
: LGTM! Efficient conditional formatting implementation.The empty string default for
title
is correctly implemented, and the conditional formatting withshowFieldName
efficiently handles the empty title case.libs/application/ui-fields/src/lib/StaticTableFormField/StaticTableFormField.tsx (1)
18-24
: LGTM! The title default value addition is well-implemented.The change aligns with the PR objective and maintains type safety while preventing undefined title values.
libs/application/ui-fields/src/lib/RadioFormField/RadioFormField.tsx (1)
35-35
: LGTM! The title default value addition is well-implemented.The change aligns with the PR objective and maintains type safety.
libs/application/templates/data-protection-complaint/src/fields/CommissionFieldRepeater/CommissionFieldRepeater.tsx (1)
29-29
: LGTM! The title default value addition is well-implemented.The change aligns with the PR objective and maintains type safety.
libs/application/ui-fields/src/lib/DateFormField/DateFormField.tsx (1)
36-36
: LGTM! The title default value addition is well-implemented.The change aligns with the PR objective and maintains type safety.
libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.tsx (1)
43-43
: LGTM! The change maintains consistency.The default empty string for
title
aligns with the PR objectives and maintains consistency with other form field components.libs/application/ui-shell/src/lib/FormShell.tsx (1)
174-174
: LGTM! Clean implementation of the default title.The implementation correctly uses the nullish coalescing operator and maintains immutability by spreading the form object.
libs/application/templates/operating-license/src/fields/PropertyRepeater/index.tsx (1)
37-37
: LGTM! Default value for title aligns with optional property pattern.The addition of a default empty string for title ensures type safety and prevents undefined values from propagating.
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (1)
51-51
: LGTM! Default value addition enhances component reliability.The default empty string for title improves the reusability of this shared UI component by preventing undefined title values.
libs/application/ui-shell/src/components/FormExternalDataProvider.tsx (3)
48-48
: LGTM! Default value in ItemHeader enhances component robustness.The addition of a default empty string for title in the ItemHeader component improves type safety.
98-98
: LGTM! Consistent default value pattern in ProviderItem.The default empty string for title maintains consistency with the optional title pattern.
166-166
: LGTM! Default value in PermissionItem follows the established pattern.The addition of a default empty string for title ensures consistent behavior across components.
libs/application/types/src/lib/Fields.ts (1)
221-221
:⚠️ Potential issueBreaking change:
title
property is now optional inBaseField
.This change makes the
title
property optional in the base interface for all form fields. While this aligns with the PR objective, it's a breaking change that could affect components that assumetitle
is always defined.Let's verify the impact of this change across the codebase:
✅ Verification successful
Title property is already handled as optional throughout the codebase
The codebase consistently handles the
title
property as potentially undefined through null checks (field.title && ...
), optional chaining, and fallback values (field.title ?? ''
). This change aligns with existing patterns and won't introduce breaking changes.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all components that might be affected by the optional title property. # Test 1: Find direct references to field.title that might not handle undefined echo "=== Direct field.title references ===" rg -A 2 'field\.title[^?]' --type ts --type tsx # Test 2: Find components that destructure title without a default value echo "=== Title destructuring without defaults ===" rg -A 2 '(?<!const \{[^}]*title = .*)\btitle\b.*\} = field' --type ts --type tsx # Test 3: Find components that pass title to formatTextWithLocale without null checks echo "=== formatTextWithLocale usage with title ===" rg -A 2 'formatTextWithLocale\([^,]*title[^,]*,' --type ts --type tsxLength of output: 799
Script:
#!/bin/bash # Configure typescript and tsx file types export RIPGREP_CONFIG_PATH=$(mktemp) echo '--type-add=ts:*.{ts,tsx}' > "$RIPGREP_CONFIG_PATH" # Find direct title access echo "=== Direct title access ===" rg 'field\.title' --type ts # Find title destructuring echo "=== Title destructuring ===" rg '\{[^}]*\btitle\b[^}]*\}\s*=\s*field' --type ts # Find title usage in components echo "=== Title usage in components ===" rg 'title[^=]*=' --type ts -g '**/*.tsx' # Find formatTextWithLocale usage echo "=== formatTextWithLocale with title ===" rg 'formatTextWithLocale.*title' --type tsLength of output: 69057
libs/application/core/src/lib/fieldBuilders.ts (1)
73-73
: LGTM! Consistent handling of optional title property.The changes correctly provide a default empty string for the
title
property, aligning with the optional title changes inBaseField
.Also applies to: 507-507, 517-517, 565-565, 569-569
libs/application/ui-fields/src/lib/DescriptionFormField/DescriptionFormField.tsx (1)
30-30
: LGTM! Safe handling of optional title.The null coalescing operator (
??
) is the appropriate choice for providing a fallback empty string whenfield.title
is undefined.libs/application/templates/operating-license/src/fields/PropertyOverviewRepeater/index.tsx (1)
21-21
: LGTM! Safe handling of optional title.The default value in the destructuring assignment is the appropriate way to handle the optional title property.
libs/application/ui-shell/src/components/FormStepper.tsx (1)
Line range hint
1-138
: Implementation follows coding guidelines effectively.The component demonstrates:
- Strong TypeScript usage with well-defined props and types
- Reusability across different NextJS apps
- Effective tree-shaking with specific imports
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.
LGTM
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.
Codeowner files lgtm
…fields (#17634) * defaulting every title to empty * chore: nx format:write update dirty files * Adding a few more optionals to titles --------- Co-authored-by: andes-it <[email protected]>
Making the title property optional
What
This is a simplified version of a PR that used to exist but got bricked during a bugged bot change, so now it should be easier to review.
Why
So that we can eventually get rid of all the (title: '') all over the place.
Summary by CodeRabbit
Based on the comprehensive summary, here's a concise overview of the changes:
Refactor
title
property across multiple components and interfacestitle
optional in many interfaces and componentstitle
to prevent undefined errorsTypes
BaseField
,Form
,FormItem
, and other interfaces to maketitle
optionalUI Components
''
) fortitle
in various form field componentsformatTextWithLocale
always receives a string for title formattingThese changes improve the robustness of title handling across the application, preventing potential undefined errors and providing more flexibility in component configurations.