-
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
fix(ui-fields): TableRepeaterField with NationalIdWithName component #15365
Conversation
WalkthroughThe changes introduce custom value mapping in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TableRepeaterFormField
participant Utils
User->>TableRepeaterFormField: Render with tableItems and values
TableRepeaterFormField->>Utils: Call handleCustomMappedValues(tableItems, values)
Utils->>Utils: Check for custom components
Utils->>TableRepeaterFormField: Return custom mapped values
TableRepeaterFormField->>User: Render with new mapped values
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15365 +/- ##
==========================================
+ Coverage 36.90% 37.05% +0.14%
==========================================
Files 6540 6541 +1
Lines 133212 133265 +53
Branches 38184 38198 +14
==========================================
+ Hits 49167 49378 +211
+ Misses 84045 83887 -158
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 453 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 8 Total Test Services: 0 Failed, 8 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (2) |
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx
Outdated
Show resolved
Hide resolved
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: 4
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- libs/application/types/src/lib/Fields.ts (2 hunks)
- libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx (3 hunks)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (4 hunks)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts (1 hunks)
Additional context used
Path-based instructions (4)
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."
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.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/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."
Biome
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
[error] 61-61: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx
[error] 103-103: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 138-139: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx
[error] 163-173: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Additional comments not posted (8)
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts (1)
1-3
: Ensure proper import paths and namespaces.Ensure that the import paths are correct and that the modules provide the expected exports. This is crucial for the file's functionality and to avoid runtime errors.
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx (2)
53-53
: Correct variable naming for consistency.The variable
nationalIdField
is correctly named to match its usage throughout the component, improving readability and maintainability.
126-126
: Ensure proper ID handling in form elements.The ID for the national ID input field is set dynamically, which is good for component reusability. Ensure that these IDs are unique within the page to avoid HTML validation issues.
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (3)
29-30
: Proper integration of utility functions and components.The import of
checkForCustomMappedComponents
andNationalIdWithName
ensures that custom components are handled correctly within the table repeater. This is crucial for the functionality of the form field.
42-42
: Correctly mappednationalIdWithName
to its component.The
nationalIdWithName
field is correctly mapped to theNationalIdWithName
component in thecomponentMapper
. This ensures that the form field can dynamically render the correct component based on the configuration.
91-92
: Check custom components during table rendering.The function
checkForCustomMappedComponents
is used to handle custom components when rendering the table. This is a good practice as it ensures that all components are rendered correctly according to their specific requirements.libs/application/types/src/lib/Fields.ts (2)
63-63
: Addition of new field typenationalIdWithName
.The new field type
nationalIdWithName
has been added toTableRepeaterFields
. This addition is consistent with the existing structure and correctly extends the type definitions to include the new field type.
118-119
: Correct definition ofnationalIdWithName
inTableRepeaterItem
.The
nationalIdWithName
component is correctly defined withinTableRepeaterItem
, ensuring it can be used effectively in the table repeater fields. This change is well-integrated with the existing type structure.
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
Outdated
Show resolved
Hide resolved
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx
Show resolved
Hide resolved
…/island-is/island.is into table_repeater_nationaldWithName
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: 4
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- libs/application/types/src/lib/Fields.ts (3 hunks)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- libs/application/types/src/lib/Fields.ts
Additional context used
Path-based instructions (1)
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."
Biome
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
[error] 60-60: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
Outdated
Show resolved
Hide resolved
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
Outside diff range and nitpick comments (1)
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (1)
Line range hint
163-173
: Optional Chaining SuggestionThe static analysis tool suggests using optional chaining to safely access nested properties. This would prevent potential runtime errors when accessing deeply nested properties if any intermediate property is undefined.
- const errors = errorList?.[activeIndex] + const errors = errorList?.[activeIndex]?.errors
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (4 hunks)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
Additional context used
Path-based instructions (1)
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."
Biome
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx
[error] 163-173: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Additional comments not posted (3)
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (3)
29-30
: Approved Import and Component Mapper ChangesThe import statements for
checkForCustomMappedComponents
andNationalIdWithName
are correctly added. TheNationalIdWithName
component is also correctly mapped in thecomponentMapper
. These changes align with the PR's objectives to integrate theNationalIdWithName
component into theTableRepeaterField
.Also applies to: 42-42
91-92
: Function Invocation CheckThe invocation of
checkForCustomMappedComponents
withtableItems
andvalues
is correctly placed within the component lifecycle, ensuring that custom components are handled dynamically based on the current state of the table items and form values. This is crucial for the dynamic component handling introduced in this PR.
298-298
: Prop Handling ValidationPassing the
application
prop to the component is appropriate as it likely needs access to the application context to function properly. This change supports the integration of theNationalIdWithName
component, which may require application-specific data to operate.
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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
…/island-is/island.is into table_repeater_nationaldWithName
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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx
Show resolved
Hide resolved
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx
Outdated
Show resolved
Hide resolved
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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (1)
Line range hint
169-179
: Use Optional Chaining for SafetyConsider using optional chaining to safely access properties.
- const tableHeader = table?.header ?? tableItems.map((item) => item.label) - const tableRows = table?.rows ?? tableItems.map((item) => item.id) + const tableHeader = table?.header ?? tableItems?.map((item) => item.label) + const tableRows = table?.rows ?? tableItems?.map((item) => item.id)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (5 hunks)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
Additional context used
Path-based instructions (1)
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."
Biome
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx
[error] 169-179: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Additional comments not posted (4)
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (4)
29-30
: Imports look good.The new imports are necessary for the added functionality.
42-42
: Component Mapper Update is Correct.The
componentMapper
now includes thenationalIdWithName
component, which is necessary for the new functionality.
230-235
: Custom Mapped Values Handling in formatTableValueThe
formatTableValue
function correctly usescustomMappedValues
if available. This ensures that custom components are properly handled.
309-309
: Passing application PropThe
application
prop is now passed to the component, which is necessary for the custom value mapping logic.
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx
Outdated
Show resolved
Hide resolved
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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (5 hunks)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
Additional context used
Path-based instructions (1)
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."
Biome
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx
[error] 164-174: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Additional comments not posted (5)
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (5)
29-30
: Imports look good.The new imports for
NationalIdWithName
andhandleCustomMappedValues
are correctly added.
42-42
: Component mapper update looks good.The
nationalIdWithName
component is correctly mapped in thecomponentMapper
.
91-92
: Custom value mapping logic looks good.The custom value mapping is correctly handled using
handleCustomMappedValues
, improving modularity and readability.
225-230
: Usage of customMappedValues looks good.The logic correctly uses
customMappedValues
if they exist, otherwise falls back tovalues
. This ensures the correct values are displayed.
304-304
: Passing application prop looks good.Passing the
application
prop to the component ensures it has the necessary context for rendering.
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: 3
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- libs/application/types/src/lib/Fields.ts (3 hunks)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (5 hunks)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- libs/application/types/src/lib/Fields.ts
Additional context used
Path-based instructions (2)
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."
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."
Biome
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx
[error] 164-174: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Additional comments not posted (4)
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (4)
29-30
: Import statement approved.The import statements for
NationalIdWithName
andhandleCustomMappedValues
are correctly added.
42-42
: Component Mapper Addition Approved.The
NationalIdWithName
component is correctly added to thecomponentMapper
.
91-92
: Custom Value Mapping Logic Approved.The custom value mapping logic using
handleCustomMappedValues
is correctly integrated.
335-335
: Prop passing approved.The
application
prop is correctly passed to the component.
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx
Show resolved
Hide resolved
…/island-is/island.is into table_repeater_nationaldWithName
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts (1 hunks)
Additional context used
Path-based instructions (1)
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."
GitHub Check: linting (application-templates-university,application-ui-fields,application-ui-shell)
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
[failure] 13-13:
'customValues' is never reassigned. Use 'const' instead
Additional comments not posted (2)
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts (2)
15-21
: Improve comments for clarity.The function
handleCustomMappedValues
could benefit from more detailed comments explaining its operations. This will help future maintainers understand the purpose and functionality of the code.- // Iterate over tableItems and handle items with nationalIdWithName component + // Iterate over tableItems and handle items with the nationalIdWithName component + // This function processes table items and applies custom value mappings for specific components.
32-34
: Improve comments for clarity.The comments explaining the special case for
nationalIdWithName
could be more detailed to enhance understanding.- // 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 is a special case where the value is an object + // with a nested object inside it. This function extracts the nested object + // and merges it with the rest of the values to ensure proper formatting.
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
Outdated
Show resolved
Hide resolved
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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/utils.ts
…15365) * fix(ui-fields): TableRepeaterField with NationalIdWithName component * tweak * tweak for item type * chore: nx format:write update dirty files * small refactor moving functions to utils * tweaks * cleanup * chore: nx format:write update dirty files * rabbit * cleanup2 * review tweaks * chore: nx format:write update dirty files * refactor * cleanup * keep handleCustomMappedValues in utils tho * tweak * added comment in utils * rabbit review * cleanup --------- Co-authored-by: andes-it <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Checklist:
Summary by CodeRabbit
New Features
nationalIdWithName
.Improvements