-
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
feat(ui-fields): update table repeater #14962
Conversation
WalkthroughThe changes introduce new functionalities to the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant TableRepeaterFormField
participant FieldBuilder
participant Messages
User->>TableRepeaterFormField: Interacts with Edit/Update Button
TableRepeaterFormField->>FieldBuilder: Calls buildTableRepeaterField with new props
FieldBuilder->>Messages: Retrieves edit/update button tooltip text
Messages-->>FieldBuilder: Returns tooltip text
FieldBuilder-->>TableRepeaterFormField: Returns configured field
TableRepeaterFormField-->>User: Renders updated field with tooltips
Tip Early Access Features
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 (
|
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 (6)
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (4)
Line range hint
147-147
: Use self-closing tags for JSX elements without children.- <T.HeadData></T.HeadData> + <T.HeadData />
Line range hint
156-166
: Consider using optional chaining for better safety.- staticData && - staticData.map((item, index) => ( + staticData?.map((item, index) => (This change ensures that the map function is called only if
staticData
is not null or undefined, preventing potential runtime errors.
Line range hint
159-159
: Use self-closing tags for JSX elements without children.- <T.Data></T.Data> + <T.Data />
Line range hint
266-266
: Prefer template literals over string concatenation.- id + 'title' + `${id}title`This change makes the code cleaner and more consistent with modern JavaScript practices.
libs/application/types/src/lib/Fields.ts (1)
Line range hint
320-320
: Specify a more appropriate type instead ofany
.Consider replacing
any
with a more specific type to improve type safety and maintainability of the code.Also applies to: 334-334
libs/application/core/src/lib/messages.ts (1)
Line range hint
782-783
: Replace unnecessary template literals with regular strings for better performance and readability.- defaultMessage: `* Umsóknartegund hefur ekki verið skilgreind\n`, + defaultMessage: '* Umsóknartegund hefur ekki verið skilgreind\n', - defaultMessage: `* Þú ert ekki með aðgang að umsókninni\n* Umsóknin er full kláruð`, + defaultMessage: '* Þú ert ekki með aðgang að umsókninni\n* Umsóknin er full kláruð', - defaultMessage: `* Það gæti verið stafsetningarvilla í umsóknarnafni\n* Umsóknin gæti verið óaðgengileg\n* Umsóknartegund gæti hafa verið eytt\n* Umsóknartegund gæti verið ógild`, + defaultMessage: '* Það gæti verið stafsetningarvilla í umsóknarnafni\n* Umsóknin gæti verið óaðgengileg\n* Umsóknartegund gæti hafa verið eytt\n* Umsóknartegund gæti verið ógild', - defaultMessage: `* Þú ert ekki með aðgang að umsókninni\n* Umsóknin gæti verið eytt\n* Umsóknin er í eigu annars notanda`, + defaultMessage: '* Þú ert ekki með aðgang að umsókninni\n* Umsóknin gæti verið eytt\n* Umsóknin er í eigu annars notanda', - defaultMessage: `* Þú hefur ekki rétt umboð til að opna þessa umsóknartegund`, + defaultMessage: '* Þú hefur ekki rétt umboð til að opna þessa umsóknartegund', - defaultMessage: `* Umsókn hefur verið fjarlægð\n* Umsókn rann út á tíma\n`, + defaultMessage: '* Umsókn hefur verið fjarlægð\n* Umsókn rann út á tíma\n',Also applies to: 799-799, 810-814, 831-834, 852-853, 874-875
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- libs/application/core/src/lib/fieldBuilders.ts (2 hunks)
- libs/application/core/src/lib/messages.ts (1 hunks)
- libs/application/types/src/lib/Fields.ts (1 hunks)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (4 hunks)
Additional Context Used
Biome (32)
libs/application/core/src/lib/fieldBuilders.ts (3)
1-44: Some named imports are only used as types.
45-46: All these imports are only used as types.
46-47: All these imports are only used as types.
libs/application/core/src/lib/messages.ts (6)
782-783: Do not use template literals if interpolation and special-character handling are not needed.
799-799: Do not use template literals if interpolation and special-character handling are not needed.
810-814: Do not use template literals if interpolation and special-character handling are not needed.
831-834: Do not use template literals if interpolation and special-character handling are not needed.
852-853: Do not use template literals if interpolation and special-character handling are not needed.
874-875: Do not use template literals if interpolation and special-character handling are not needed.
libs/application/types/src/lib/Fields.ts (13)
320-320: Unexpected any. Specify a different type.
334-334: Unexpected any. Specify a different type.
10-11: All these imports are only used as types.
11-12: All these imports are only used as types.
12-13: All these imports are only used as types.
13-14: All these imports are only used as types.
14-15: All these imports are only used as types.
15-16: All these imports are only used as types.
16-17: All these imports are only used as types.
17-18: All these imports are only used as types.
18-19: All these imports are only used as types.
19-20: All these imports are only used as types.
20-21: All these imports are only used as types.
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (10)
147-147: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
156-166: Change to an optional chain.
159-159: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
266-266: Template literals are preferred over string concatenation.
1-5: All these imports are only used as types.
26-27: Some named imports are only used as types.
149-149: Avoid using the index of an array as key property in an element.
158-158: Avoid using the index of an array as key property in an element.
161-161: Avoid using the index of an array as key property in an element.
216-216: Avoid using the index of an array as key property in an element.
Path-based Instructions (4)
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."
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/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."
Additional comments not posted (6)
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (3)
61-62
: Ensure default values align with expected behavior.The default values for
updateButtonTooltipText
andupdateFields
are set appropriately according to the PR description.
108-111
: EnsurehandleChangeItem
is robust and handles all edge cases.The function
handleChangeItem
is simple and correctly sets the active index for editing a row. Ensure it handles all edge cases, especially with asynchronous data updates.
191-212
: Review the conditional rendering logic for the update button.The conditional rendering of the update button based on the
updateFields
prop is implemented correctly. The tooltip is also appropriately set usingupdateButtonTooltipText
.libs/application/types/src/lib/Fields.ts (1)
503-504
: Ensure new properties are optional in interfaces.The optional chaining for
updateButtonTooltipText
andupdateFields
inTableRepeaterField
is correctly implemented, making these properties optional and aligning with the PR description.libs/application/core/src/lib/fieldBuilders.ts (1)
Line range hint
745-765
: Review the implementation ofbuildTableRepeaterField
.The function
buildTableRepeaterField
correctly integrates the new propertiesupdateButtonTooltipText
andupdateFields
. This aligns with the PR objectives and enhances the functionality of the table repeater fields.libs/application/core/src/lib/messages.ts (1)
223-227
: The addition of theupdateFieldText
message is correctly implemented and aligns with the existing message structure.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14962 +/- ##
==========================================
+ Coverage 37.13% 37.16% +0.03%
==========================================
Files 6373 6374 +1
Lines 129705 129780 +75
Branches 36998 37041 +43
==========================================
+ Hits 48163 48236 +73
- Misses 81542 81544 +2 Flags with carried forward coverage won't be shown. Click here to find out more.
... and 67 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 9 Total Test Services: 0 Failed, 9 Passed Test Services
|
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.
Nice!
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 (5)
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (4)
61-62
: The default values foreditButtonTooltipText
andeditField
are well-integrated with the existing structure. However, consider adding a comment to explain whyeditField
defaults tofalse
, as it might not be immediately clear to other developers.
Line range hint
147-147
: JSX elements without children should be self-closing to improve readability and adhere to best practices.- <T.HeadData></T.HeadData> + <T.HeadData /> - <T.Data></T.Data> + <T.Data />Also applies to: 159-159
Line range hint
156-166
: Consider using optional chaining to safely access properties on potentially undefined objects.- {staticData && staticData.map((item, index) => ( + {staticData?.map((item, index) => (
Line range hint
266-266
: Prefer template literals over string concatenation for better readability and consistency.- id + 'title' + `${id}title`libs/application/core/src/lib/messages.ts (1)
Line range hint
782-783
: Consider replacing template literals with regular strings where interpolation and special-character handling are not needed. This can improve performance slightly and reduce complexity.- defaultMessage: `* Þú ert ekki með aðgang að umsókninni\n* Umsóknin gæti verið eytt\n* Umsóknin er í eigu annars notanda`, + defaultMessage: '* Þú ert ekki með aðgang að umsókninni\n* Umsóknin gæti verið eytt\n* Umsóknin er í eigu annars notanda',Apply similar changes to other lines mentioned in the static analysis hints.
Also applies to: 799-799, 810-814, 831-834, 852-853, 874-875
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- libs/application/core/src/lib/fieldBuilders.ts (2 hunks)
- libs/application/core/src/lib/messages.ts (1 hunks)
- libs/application/types/src/lib/Fields.ts (1 hunks)
- libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (4 hunks)
Additional Context Used
Biome (32)
libs/application/core/src/lib/fieldBuilders.ts (3)
1-44: Some named imports are only used as types.
45-46: All these imports are only used as types.
46-47: All these imports are only used as types.
libs/application/core/src/lib/messages.ts (6)
782-783: Do not use template literals if interpolation and special-character handling are not needed.
799-799: Do not use template literals if interpolation and special-character handling are not needed.
810-814: Do not use template literals if interpolation and special-character handling are not needed.
831-834: Do not use template literals if interpolation and special-character handling are not needed.
852-853: Do not use template literals if interpolation and special-character handling are not needed.
874-875: Do not use template literals if interpolation and special-character handling are not needed.
libs/application/types/src/lib/Fields.ts (13)
320-320: Unexpected any. Specify a different type.
334-334: Unexpected any. Specify a different type.
10-11: All these imports are only used as types.
11-12: All these imports are only used as types.
12-13: All these imports are only used as types.
13-14: All these imports are only used as types.
14-15: All these imports are only used as types.
15-16: All these imports are only used as types.
16-17: All these imports are only used as types.
17-18: All these imports are only used as types.
18-19: All these imports are only used as types.
19-20: All these imports are only used as types.
20-21: All these imports are only used as types.
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (10)
147-147: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
156-166: Change to an optional chain.
159-159: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
266-266: Template literals are preferred over string concatenation.
1-5: All these imports are only used as types.
26-27: Some named imports are only used as types.
149-149: Avoid using the index of an array as key property in an element.
158-158: Avoid using the index of an array as key property in an element.
161-161: Avoid using the index of an array as key property in an element.
216-216: Avoid using the index of an array as key property in an element.
Path-based Instructions (4)
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."
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/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."
Additional comments not posted (4)
libs/application/ui-fields/src/lib/TableRepeaterFormField/TableRepeaterFormField.tsx (1)
108-110
: ThehandleEditItem
function is concise and clear. It effectively sets the active index for editing. Ensure that the index handling is robust against possible out-of-bound errors.libs/application/types/src/lib/Fields.ts (1)
503-504
: The addition ofeditButtonTooltipText
andeditField
properties toTableRepeaterField
aligns with the new functionality. Ensure that these properties are documented in the type definition to help developers understand their usage.libs/application/core/src/lib/fieldBuilders.ts (1)
Line range hint
745-765
: ThebuildTableRepeaterField
function has been updated to include parameters for editing functionality. This is a crucial update for the feature. Ensure that all new parameters are used appropriately within the function and that their interactions with existing parameters are tested.libs/application/core/src/lib/messages.ts (1)
223-227
: The new message definitions foreditFieldText
are correctly implemented and follow the existing pattern in the file.
What
Add edit button to a table row in table repeater component
Why
To be able to edit content of a table row instead of needing to delete and add again.
Edit button is optional and is set using
editField: true
. Hover text can be changes usingeditButtonTooltipText
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
New Features
TableRepeaterFormField
component.Improvements
TableRepeaterFormField
component with new props for better customization and functionality.