feat(application-system): new shared component buildFieldsRepeaterField #16871
Conversation
WalkthroughThe changes in this pull request introduce a new field builder function, 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
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16871 +/- ##
==========================================
- Coverage 35.59% 35.55% -0.04%
==========================================
Files 6924 6926 +2
Lines 146399 146557 +158
Branches 41573 41654 +81
==========================================
+ Hits 52105 52114 +9
- Misses 94294 94443 +149 Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
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 (3) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (20)
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts (2)
Line range hint
10-19: Add JSDoc documentation for better clarity.While the function implementation is good, it would benefit from JSDoc documentation explaining:
- The purpose of the transformation
- The expected input format
- The return value structure
- Why certain components (like 'nationalIdWithName') receive special handling
+/** + * Transforms an array of table items and their corresponding values, + * applying special handling for items with 'nationalIdWithName' component type. + * @param tableItems - Array of items with component information + * @param values - Array of corresponding values for each item + * @returns Transformed array of values with special handling applied + */ export const handleCustomMappedValues = <T>( tableItems: Array<Item>, values: Array<Value<T>>,
Line range hint
21-41: Consider adding error handling for malformed data.While the function handles basic edge cases, it could benefit from additional error handling:
- Validate the structure of nested objects
- Handle potential type mismatches
- Log or report malformed data
const handleNationalIdWithNameItem = <T>( item: Item, values: Array<Value<T>>, ) => { if (!values?.length) { return [] } const newValues = values.map((value) => { + if (!(item.id in value)) { + console.warn(`Missing expected field ${item.id} in value object`) + return value + } if (typeof value[item.id] === 'object' && value[item.id] !== null) { const { [item.id]: nestedObject, ...rest } = value + if (!nestedObject || typeof nestedObject !== 'object') { + console.warn(`Invalid nested object for ${item.id}`) + return value + } return { ...nestedObject, ...rest } } return value })libs/application/ui-fields/src/lib/FieldsRepeaterFormField/utils.ts (2)
26-42: Improve type safety and add context to commentsThe implementation could benefit from better type safety and documentation:
Consider these improvements:
const handleNationalIdWithNameItem = <T>( item: Item, values: Array<Value<T>>, ) => { if (!values?.length) { return [] } - // nationalIdWithName is a special case where the value is an object - // with a nested object inside it. This function will extract the nested - // object and merge it with the rest of the values. + // The nationalIdWithName component stores its data in a nested structure + // to maintain separation between the national ID and name fields. + // This transformation flattens the structure to make it consistent + // with other field types in the form. const newValues = values.map((value) => { if (typeof value[item.id] === 'object' && value[item.id] !== null) { const { [item.id]: nestedObject, ...rest } = value + // Ensure nestedObject is of type T + if (!isValidNestedObject<T>(nestedObject)) { + return value + } return { ...nestedObject, ...rest } } return value }) return newValues } + +// Type guard to ensure nested object matches expected structure +function isValidNestedObject<T>(obj: unknown): obj is T { + return obj !== null && typeof obj === 'object' + // Add more specific checks based on T's expected structure +}
1-42: Consider architectural improvements for better reusabilityAs this code resides in the
libsdirectory, consider these architectural improvements:
- Extract the component-specific logic into a pluggable handler system.
- Define an interface for custom field handlers that other NextJS apps can implement.
- Consider moving the field-specific transformations to their respective component files.
This would improve reusability across different NextJS apps while maintaining clear separation of concerns.
libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.stories.mdx (4)
35-37: Add validation exampleThe documentation mentions zod schema validation but doesn't provide an example. Consider adding a code snippet demonstrating the validation implementation.
Would you like me to provide an example of zod schema validation for this repeater field?
74-77: Consider improving phone format regexThe current phone formatting regex is simplistic. Consider using a more robust pattern that handles different phone number formats.
- phone: (value) => value.replace(/^(.{3})/, '$1-'), + phone: (value) => { + // Remove non-digits first + const cleaned = value.replace(/\D/g, ''); + // Format as XXX-XXXX + return cleaned.replace(/^(\d{3})(\d{4})$/, '$1-$2'); + },
83-127: Apply DRY principle to field configurationThe field configuration is duplicated between the example and the story. Consider extracting it into a shared constant.
const exampleFieldConfig = { id: 'field.id', title: 'My repeater', // ... rest of the configuration }; // Use in Source component <Source code={dedent(`buildTableRepeaterField(${JSON.stringify(exampleFieldConfig, null, 2)})`)} /> // Use in Story <Story name="Default"> <TableRepeaterFormField application={createMockApplication()} field={exampleFieldConfig} /> </Story>
129-129: Fix grammar: Replace "into" with "in"Change the preposition for better readability.
-You can also use this field into a custom component by using `<TableRepeaterFormField field={...} />` +You can also use this field in a custom component by using `<TableRepeaterFormField field={...} />`🧰 Tools
🪛 LanguageTool
[uncategorized] ~129-~129: The preposition “in” seems more likely in this position than the preposition “into”.
Context: ... You can also use this field into a custom component by using `<TableRepe...(AI_EN_LECTOR_REPLACEMENT_PREPOSITION_INTO_IN)
libs/application/core/src/lib/fieldBuilders.ts (1)
867-897: Consider enhancing reusability through shared utilitiesThe implementation largely duplicates code from
buildTableRepeaterField. Consider extracting shared logic into a utility function to improve maintainability.Example refactor:
const buildBaseRepeaterField = (data: BaseRepeaterConfig, type: FieldTypes, component: FieldComponents) => ({ ...extractCommonFields(data), children: undefined, type, component, fields: data.fields, table: data.table, formTitle: data.formTitle, marginTop: data.marginTop, marginBottom: data.marginBottom, titleVariant: data.titleVariant, addItemButtonText: data.addItemButtonText, saveItemButtonText: data.saveItemButtonText, maxRows: data.maxRows, }) export const buildFieldsRepeaterField = ( data: Omit<FieldsRepeaterField, 'type' | 'component' | 'children'>, ): FieldsRepeaterField => buildBaseRepeaterField(data, FieldTypes.FIELDS_REPEATER, FieldComponents.FIELDS_REPEATER)libs/application/types/src/lib/Fields.ts (1)
Line range hint
787-827: Add FieldsRepeaterField to the Field union typeThe new
FieldsRepeaterFieldtype needs to be added to theFieldunion type to be usable in the application.Add the new type to the union:
export type Field = | CheckboxField | CustomField ... | TableRepeaterField + | FieldsRepeaterField | HiddenInputWithWatchedValueField ... | SliderFieldlibs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.tsx (3)
9-9: Remove unused import for effective tree-shakingThe imported
TableRepeaterFieldis not used in this file. Removing unused imports improves tree-shaking and reduces the bundle size.
18-18: Remove unused import aliasTable as TThe alias
Table as Tis imported but not utilized in the component. Eliminating unused imports enhances code clarity and bundling efficiency.
104-110: Remove unused functionformatTableValueThe function
formatTableValueis defined but not called anywhere in the code. Removing unused functions improves maintainability and reduces code complexity.libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterItem.tsx (5)
162-185: Refactor default value assignment to reduce code duplicationThe default value assignment logic for different components contains repetitive code blocks, which can be simplified.
Create a helper function to handle default value retrieval, reducing duplication and enhancing readability.
Example:
const getComponentDefaultValue = () => { return ( getValueViaPath(application.answers, id) ?? getDefaultValue(item, application, activeValues) ) } let DefaultValue = getComponentDefaultValue()Then, use
DefaultValuefor components that support it.
70-86: OptimizewatchedValuescomputation usinguseMemoCalculating
watchedValuescan be performance-intensive if the dependencies are complex.Use
useMemoto memoizewatchedValues, preventing unnecessary recalculations and improving performance.Example:
const watchedValues = useMemo(() => { if (!updateValueObj) return undefined const watchedValuesId = typeof updateValueObj.watchValues === 'function' ? updateValueObj.watchValues(activeValues) : updateValueObj.watchValues if (watchedValuesId) { if (Array.isArray(watchedValuesId)) { return watchedValuesId.map((value) => activeValues?.[`${value}`]) } else { return activeValues?.[`${watchedValuesId}`] } } }, [updateValueObj, activeValues])
148-160: SimplifyreadonlyanddisabledevaluationsThe evaluations for
ReadonlyandDisabledprops have similar structures that can be consolidated.Create a utility function to handle these evaluations, reducing code duplication.
Example:
const evaluateProp = ( prop: boolean | ((application: Application, values?: Record<string, string>) => boolean), ) => { return typeof prop === 'function' ? prop(application, activeValues) : prop } const Readonly = evaluateProp(readonly) const Disabled = evaluateProp(disabled)
191-220: Ensure consistency insplitprop handlingThe
splitprop on line 204 handles half widths but not third widths, potentially causing layout inconsistencies.Update the
splitprop to handle'third'width for consistency with thespancalculation.- split={width === 'half' ? '1/2' : '1/1'} + split={width === 'half' ? '1/2' : width === 'third' ? '1/3' : '1/1'}
121-133: Handle undefineddefaultValuemore gracefullyIn the
getDefaultValuefunction, ifdefaultValueis undefined, the function returnsundefined, but it might be clearer to have an explicit return value.Consider simplifying the function:
const getDefaultValue = ( item: RepeaterItem, application: Application, activeField?: Record<string, string>, ) => { if (!item.defaultValue) return undefined return item.defaultValue(application, activeField) }libs/application/templates/reference-template/src/forms/ExampleForm.ts (2)
63-63: Possible typo in label 'Radio' for the 'ratio' fieldThe label for the
ratiofield is set to'Radio'(lines 63 and 95), but the field appears to represent a percentage ratio. If'Ratio'was intended, please correct the label to avoid confusion.Also applies to: 95-95
66-71: Extract duplicated option generation code into a reusable functionThe code for generating options for the
ratiofield is duplicated in both repeater fields (lines 66-71 and 98-103). Consider extracting this logic into a helper function or utility to enhance code reusability and maintainability.Also applies to: 98-103
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (8)
libs/application/core/src/lib/fieldBuilders.ts(2 hunks)libs/application/templates/reference-template/src/forms/ExampleForm.ts(2 hunks)libs/application/types/src/lib/Fields.ts(6 hunks)libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.stories.mdx(1 hunks)libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.tsx(1 hunks)libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterItem.tsx(1 hunks)libs/application/ui-fields/src/lib/FieldsRepeaterFormField/utils.ts(1 hunks)libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
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/FieldsRepeaterFormField/FieldsRepeaterFormField.stories.mdx (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/FieldsRepeaterFormField/FieldsRepeaterItem.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/utils.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/TableRepeaterFormField/utils.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."
🪛 LanguageTool
libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.stories.mdx
[uncategorized] ~78-~78: Loose punctuation mark.
Context: ...})/, '$1-'), }, }, })`)} /> The previous configuration object ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~129-~129: The preposition “in” seems more likely in this position than the preposition “into”.
Context: ... You can also use this field into a custom component by using `<TableRepe...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION_INTO_IN)
🔇 Additional comments (8)
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts (2)
8-8: LGTM! Well-defined generic type.
The Value<T> type is properly exported and uses TypeScript generics effectively for type safety.
1-5: LGTM! Verify type change impact.
The type definition and import look good. The change from TableRepeaterItem to RepeaterItem follows TypeScript best practices and promotes reusability.
Let's verify the impact of this type change:
libs/application/ui-fields/src/lib/FieldsRepeaterFormField/utils.ts (2)
1-8: LGTM! Well-structured type definitions
The type definitions follow TypeScript best practices with proper generic typing and clear interface extension.
22-25: LGTM! Well-typed function signature
The function declaration follows TypeScript best practices with proper generic typing and parameter definitions.
libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.stories.mdx (2)
1-24: LGTM! Well-structured imports and mock data generation
The implementation follows TypeScript best practices with proper type exports and the mock data generator is well-designed for reusability.
26-32: LGTM! Clear component categorization
The Storybook metadata is well-organized and follows the component hierarchy.
libs/application/types/src/lib/Fields.ts (1)
67-67: LGTM! Good type renaming for better reusability
The renaming from TableRepeater to Repeater makes these types more generic and reusable across different repeater implementations.
Also applies to: 85-85
libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.tsx (1)
40-52: Ensure all destructured properties exist on FieldsRepeaterField type
Verify that all the destructured properties from data, such as fields, table, formTitle, etc., are properly defined in the FieldsRepeaterField type. This ensures type safety and prevents potential runtime errors.
libs/application/ui-fields/src/lib/FieldsRepeaterFormField/utils.ts
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.tsx
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.tsx
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterItem.tsx
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterItem.tsx
Outdated
Show resolved
Hide resolved
libs/application/templates/reference-template/src/forms/ExampleForm.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (7)
libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.tsx (4)
26-28: Consider exporting the Props interfaceSince this component is in the libs directory, exporting the Props interface would improve reusability and allow other components to reference these types.
-interface Props extends FieldBaseProps { +export interface FieldsRepeaterFormFieldProps extends FieldBaseProps { field: FieldsRepeaterField }
30-47: Add JSDoc documentation for the componentAdding JSDoc documentation would improve maintainability and help other developers understand the component's purpose and props.
+/** + * A form field component that allows users to dynamically add and remove sets of fields. + * @param {Object} props - Component props + * @param {Object} props.application - The current application context + * @param {FieldsRepeaterField} props.field - Field configuration + * @param {boolean} props.showFieldName - Whether to show the field name + * @param {string} props.error - Error message if validation fails + */ export const FieldsRepeaterFormField = ({
110-121: Consider memoizing repeaterFields functionThe repeaterFields function creates new components on every render. Consider using useMemo to optimize performance, especially with large forms.
-const repeaterFields = (index: number) => +const repeaterFields = React.useMemo( + () => (index: number) => items.map((item) => ( <Item key={`${id}[${activeIndex}].${item.id}.${index}`} application={updatedApplication} error={error} item={item} dataId={id} index={index} values={values} /> )) + [id, activeIndex, updatedApplication, error, items, values] +)
1-1: Add component documentation fileSince this is a library component, consider adding a README.md file in the same directory with:
- Component description
- Usage examples
- Props documentation
- Common use cases
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterItem.tsx (1)
124-126: Consider enhancing type safety for the return value.The function signature update looks good. However, consider adding a return type annotation to make the function signature more explicit and type-safe.
const getDefaultValue = ( item: RepeaterItem, application: Application, activeField?: Record<string, string>, - ) => { + ): string | string[] | undefined => {libs/application/templates/reference-template/src/forms/ExampleForm.ts (2)
66-71: Extract ratio options generation for reusabilityThe ratio options generation logic is duplicated. Consider extracting it into a reusable function to improve maintainability and reduce code duplication.
const generateRatioOptions = () => Array(100) .fill(undefined) .map((_, idx, array) => ({ value: `${array.length - idx}`, label: `${array.length - idx}%`, }));Then use it in both repeaters:
- options: Array(100) - .fill(undefined) - .map((_, idx, array) => ({ - value: `${array.length - idx}`, - label: `${array.length - idx}%`, - })), + options: generateRatioOptions(),Also applies to: 99-104
46-73: Extract common field configurationsThe field configurations are duplicated between the two repeaters. Consider extracting them into a shared configuration object to improve maintainability and reduce duplication.
const commonFields = { email: { component: 'input', label: 'Email', type: 'email', dataTestId: 'employer-email', }, phoneNumber: { component: 'input', label: 'Phonenumber', type: 'tel', format: '###-####', placeholder: '000-0000', dataTestId: 'employer-phone-number', }, ratio: { component: 'select', label: 'Radio', placeholder: 'placeholder', dataTestId: 'employment-ratio', options: generateRatioOptions(), }, };Then use it in both repeaters:
- fields: { - email: { ... }, - phoneNumber: { ... }, - ratio: { ... }, - }, + fields: commonFields,Also applies to: 79-106
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (8)
libs/application/core/src/lib/fieldBuilders.ts(2 hunks)libs/application/core/src/lib/messages.ts(1 hunks)libs/application/templates/reference-template/src/forms/ExampleForm.ts(2 hunks)libs/application/types/src/lib/Fields.ts(7 hunks)libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.tsx(1 hunks)libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterItem.tsx(1 hunks)libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterItem.tsx(3 hunks)libs/application/ui-fields/src/lib/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/application/core/src/lib/fieldBuilders.ts
- libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterItem.tsx
🧰 Additional context used
📓 Path-based instructions (6)
libs/application/core/src/lib/messages.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/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/TableRepeaterFormField/TableRepeaterItem.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/index.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (8)
libs/application/ui-fields/src/lib/index.ts (2)
27-27: LGTM! Verify component implementation.
The new export follows the established pattern and is positioned logically near similar components.
Let's verify the component implementation:
#!/bin/bash
# Description: Verify the existence and implementation of FieldsRepeaterFormField
# Check if the component file exists and show its content
fd "FieldsRepeaterFormField.tsx?" --type f --exec cat {}27-27: Verify TypeScript types for the new component.
Since this is part of a shared UI library, please ensure proper TypeScript types are defined and exported for the FieldsRepeaterFormField component.
Let's check the type definitions:
✅ Verification successful
TypeScript types are properly defined for the component
The component has proper TypeScript types defined:
- The component extends
FieldBasePropsimported from '@island.is/application/types' - A clear interface
Propsis defined that extends the base props - The props interface includes proper typing for the required
fieldprop asFieldsRepeaterField - All props are properly typed in the component's parameter destructuring
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check TypeScript types for the FieldsRepeaterFormField component
# Look for type definitions in the component file
ast-grep --pattern 'interface $name {
$$$
}'
# Look for type exports
ast-grep --pattern 'export (type|interface) $name {
$$$
}'
Length of output: 111
Script:
#!/bin/bash
# Let's try a different approach to find type definitions
# First, let's locate the component file
fd "FieldsRepeaterFormField.tsx?" --type f
# Then let's examine its contents for type definitions
rg -A 2 "type|interface" --type ts --type tsx
# Also check for Props type specifically
rg -A 2 "Props" --type ts --type tsx
# Look for any exported types
rg "export.*type|export.*interface" --type ts --type tsx
Length of output: 359
Script:
#!/bin/bash
# Let's try without file type restrictions since tsx isn't recognized
# Examine the component file directly
cat libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.tsx
# Backup search for type definitions
rg "type|interface|Props" libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.tsx
Length of output: 5634
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterItem.tsx (2)
2-2: LGTM! Import change aligns with the generic repeater approach.
The change from TableRepeaterItem to RepeaterItem type promotes better reusability across different repeater implementations.
21-21: Verify the impact of type changes on consumers.
The type change from TableRepeaterItem to RepeaterItem looks good, but let's verify its usage across the codebase.
✅ Verification successful
Type change from TableRepeaterItem to RepeaterItem is safe to proceed
Based on the codebase analysis:
- The
RepeaterItemtype is the base type defined inlibs/application/types/src/lib/Fields.ts - The change aligns with the import from
@island.is/application/types - No direct usage of
TableRepeaterItemtype was found in the codebase - The component maintains the
& { id: string }requirement, preserving type safety
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components using ItemFieldProps or TableRepeaterItem
rg -l "ItemFieldProps|TableRepeaterItem" --type typescript
# Search for direct usage of the item prop with TableRepeaterItem type
ast-grep --pattern 'item: TableRepeaterItem'
Length of output: 142
Script:
#!/bin/bash
# Search for components using ItemFieldProps or TableRepeaterItem
rg -l "ItemFieldProps|TableRepeaterItem"
# Search for direct usage of the item prop with TableRepeaterItem type
rg -A 2 "item:\s+TableRepeaterItem"
# Search for files importing TableRepeaterItem
rg "import.*TableRepeaterItem"
# Search for any usage of TableRepeaterItem in type definitions
rg ":\s*TableRepeaterItem"
Length of output: 517
Script:
#!/bin/bash
# Check the content of TableRepeaterFormField.tsx to see how it uses the Item type
cat libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx
# Check the content of FieldsRepeaterItem.tsx to understand related implementations
cat libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterItem.tsx
# Look for RepeaterItem usage
rg "RepeaterItem"
Length of output: 26882
libs/application/templates/reference-template/src/forms/ExampleForm.ts (1)
35-111: Implementation follows best practices
The new repeater section follows the established patterns and best practices:
- Maintains proper TypeScript typing
- Uses modular and reusable components
- Follows the existing form structure
libs/application/types/src/lib/Fields.ts (2)
800-800: LGTM!
The FieldsRepeaterField type is correctly added to the Field union type.
256-256: Verify component implementation exists
The new field type and component mapping look correct. However, ensure that the FieldsRepeaterFormField component exists and is properly implemented.
Also applies to: 292-292
✅ Verification successful
Component implementation verified successfully
The FieldsRepeaterFormField component exists and is properly implemented with:
- Correct props handling and form integration
- Field array management with add/remove functionality
- Error handling and validation
- Proper rendering of repeatable fields
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the FieldsRepeaterFormField component exists
fd -e tsx -e ts "FieldsRepeaterFormField"
Length of output: 128
Script:
#!/bin/bash
# Check if FieldsRepeaterFormField is properly implemented by looking for key component patterns
ast-grep --pattern 'export const FieldsRepeaterFormField = $_'
Length of output: 18827
libs/application/core/src/lib/messages.ts (1)
44-48: LGTM! The new message definition follows established patterns.
The new buttonRemove message is well-structured and consistent with other button messages in terms of:
- Message key naming
- ID format
- Description clarity
- Integration with existing messages
libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.tsx
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterFormField.tsx
Outdated
Show resolved
Hide resolved
libs/application/templates/reference-template/src/forms/ExampleForm.ts
Outdated
Show resolved
Hide resolved
libs/application/templates/reference-template/src/forms/ExampleForm.ts
Outdated
Show resolved
Hide resolved
obmagnusson
changed the title
jonnigs
added
deploy-feature
jonnigs
changed the title
jonnigs
added
the
deprecated:automerge
RunarVestmann
added
deprecated:automerge
…ld (#16871) * feat: start of fieldsRepeaterField * feat: Make a copy of table repeater and update naming * feat: make sure to throw away answers when repeater item is moved. * . * feat: additional options for fieldsRepeater * chore: remove console.log * feat: handle edge case going back and forth * chore: revert back changes to the example form * feat: update storybook --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
Make a repeater similar to table repeater but without the table
Why
This is a functionality that is needed and the old
buildRepeaterdoesn't handle this well enoughScreenshots / Gifs
Checklist:
Summary by CodeRabbit
New Features
FieldsRepeaterFieldcomponent for dynamic form management, allowing users to add, edit, and remove items.ExampleFormwith a "Test repeater" section for user input of multiple entries.TableRepeaterFormFieldto guide users in implementation.Bug Fixes
Refactor