Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/helpers.js (1)
465-487:⚠️ Potential issue | 🟠 Major
isRemovedhandling is order-dependent and can re-introduce removed fields.At Line 469, removal is applied only at that iteration point. If the same
nameappears later incustomFieldswith aposition, the field can be added back.isRemovedshould win deterministically for a field name, independent of entry order.Proposed fix (make removals deterministic)
export const makeFields = (customFields, defaultFields) => { if (!customFields) { return defaultFields } - let fields = [...defaultFields] + const removedFieldNames = new Set( + customFields.filter(field => field.isRemoved).map(field => field.name) + ) + let fields = defaultFields.filter( + field => !removedFieldNames.has(field.name) + ) customFields.forEach(customField => { + if (removedFieldNames.has(customField.name)) { + return + } + const defaultField = fields.find(field => field.name === customField.name) - // If isRemoved is true, remove the field from the result - if (customField.isRemoved) { - if (defaultField) { - fields = fields.filter(field => field.name !== customField.name) - } - return - } - const _field = defaultField || customField🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/helpers.js` around lines 465 - 487, Detect removed names up-front and make removals deterministic by computing a Set of names to remove from customFields (e.g., const removedNames = new Set(customFields.filter(cf => cf.isRemoved).map(cf => cf.name))) before processing, remove any matching entries from fields immediately, and then in the customFields iteration skip any customField whose name is in removedNames (so later entries with positions cannot re-introduce them); update logic around customFields/fields/isRemoved and the merge into _field to respect this precomputed removedNames set.
🧹 Nitpick comments (5)
AGENTS.md (1)
29-39: Make import-order guidance exact to avoid interpretation drift.This section is close, but still descriptive. Since this file is operational guidance for generators, include the exact
pathGroupssnippet so behavior is unambiguous (especially cozy/twake placement anddistinctGroupimpact).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 29 - 39, Update the Import/Order section to include the exact ESLint `pathGroups` configuration so the generator behavior is unambiguous: state the `alphabetize: { order: 'asc' }`, `newlines-between: 'always'`, the `groups` order (`builtin`, `external`, `internal`, `['parent','sibling','index']`), and provide the exact `pathGroups` entries for `cozy-*` and `twake-*` plus the `distinctGroup` setting; place these identifiers (`pathGroups`, `alphabetize`, `newlines-between`, `groups`, `distinctGroup`, `cozy-*`, `twake-*`) inline in the doc so implementers can copy them verbatim into ESLint config without interpretation drift.packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/helpers.spec.js (1)
484-505: Add a regression test for duplicate field names with conflicting entries.Please add a case where the same
nameappears twice (oneisRemoved: true, one withposition) to assert deterministic precedence (removed should stay removed regardless of order).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/helpers.spec.js` around lines 484 - 505, Add a regression test in the ContactForm helpers.spec.js that covers duplicate custom field names with conflicting entries: call makeFields with defaultFields and a customFields array containing two entries for the same name (one entry having isRemoved: true and another having position or other overrides), ensure the resulting array does not include that field (i.e., the isRemoved entry takes precedence regardless of order), and add assertions for both input orders (removed-first and removed-second) to guarantee deterministic behavior of makeFields.packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInput.jsx (1)
124-142: Declare the newshowRemove/onRemoveprops inpropTypes(and defaults).These props are now part of
FieldInputbehavior but are not typed in the component contract.Suggested change
FieldInput.propTypes = { name: PropTypes.string.isRequired, className: PropTypes.string, labelProps: labelPropTypes, attributes: fieldInputAttributesTypes, + showRemove: PropTypes.bool, + onRemove: PropTypes.func, /** Whether the field is visible by the user or not (still in the DOM anyway) */ isInvisible: PropTypes.bool, contacts: PropTypes.shape({ data: PropTypes.arrayOf(PropTypes.object) }), @@ FieldInput.defaultProps = { - labelProps: null + labelProps: null, + showRemove: false, + onRemove: undefined }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInput.jsx` around lines 124 - 142, Add the missing showRemove and onRemove props to FieldInput's contract: update FieldInput.propTypes to include showRemove: PropTypes.bool and onRemove: PropTypes.func, and add default values in FieldInput.defaultProps (e.g., showRemove: false and onRemove: null) so the component declares the new optional boolean flag and the optional callback handler; modify the existing FieldInput.propTypes and FieldInput.defaultProps blocks accordingly.packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/index.jsx (1)
97-100: Use a stable field key instead of array index.Line 99 uses
indexas key; this can cause incorrect component state retention if field order changes.Suggested change
- {_fields.map((attributes, index) => ( + {_fields.map((attributes, index) => ( <Box - key={index} + key={attributes.name || index} width={🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/index.jsx` around lines 97 - 100, The map in ContactForm rendering uses the array index as the key for each Box (the _fields.map callback); replace the unstable key with a stable unique identifier from each field object (e.g., attributes.id, attributes.key, or attributes.name) so React can correctly preserve component state when field order changes—update the key prop on the Box in the _fields.map to use that stable identifier (or generate/store a persistent id when fields are created) instead of index.packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInputWrapper.jsx (1)
41-41: Avoid forcingsize="small"when a field explicitly defines size.Line 41 currently overrides any upstream
sizepassed via attributes/props, which removes per-field control.Suggested change
- size="small" + size={props.size ?? restAttributes.size ?? 'small'}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInputWrapper.jsx` at line 41, FieldInputWrapper currently forces size="small" which overrides any size passed from parent props; update FieldInputWrapper.jsx (the FieldInputWrapper component) to stop hard-coding size and instead respect an incoming size prop (e.g., use the passed-in size if present, otherwise default to "small"), or spread {...props} so per-field size controls are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Line 37: The package pattern in the ESLint pathGroup was written as "cozy-_"
and "twake-_" which are typos; update those patterns to use wildcard notation
"cozy-*" and "twake-*" so they match packages like cozy-<name> and twake-<name>
(locate the entry referencing cozy-_ / twake-_ in AGENTS.md or the ESLint
pathGroup configuration and replace the underscore with an asterisk).
- Around line 53-73: The example code block uses double quotes and semicolons
but the document requires single quotes and no semicolons; update the snippet
for MyComponent (and its imports: React, cx, Button, FieldInput,
makeIsRequiredError) to use single quotes everywhere and remove all trailing
semicolons, preserving the same JSX structure and variable names (MyComponent,
isError, makeIsRequiredError, FieldInput, cx) so the example matches the
documented style rules.
In `@packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/RemoveButton.jsx`:
- Line 11: The IconButton in RemoveButton.jsx currently uses a hardcoded English
aria-label ("delete"); replace this with a localized string obtained from the
app's i18n system (e.g. use the existing translation hook or helper in scope)
and pass that value to aria-label so screen readers receive the translated text;
locate the IconButton component in RemoveButton.jsx (the element with
onClick={onRemove}) and swap the literal aria-label for the translated key (e.g.
t('contacts.delete') or the appropriate namespace) ensuring fallback/defaults
are preserved.
---
Outside diff comments:
In `@packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/helpers.js`:
- Around line 465-487: Detect removed names up-front and make removals
deterministic by computing a Set of names to remove from customFields (e.g.,
const removedNames = new Set(customFields.filter(cf => cf.isRemoved).map(cf =>
cf.name))) before processing, remove any matching entries from fields
immediately, and then in the customFields iteration skip any customField whose
name is in removedNames (so later entries with positions cannot re-introduce
them); update logic around customFields/fields/isRemoved and the merge into
_field to respect this precomputed removedNames set.
---
Nitpick comments:
In `@AGENTS.md`:
- Around line 29-39: Update the Import/Order section to include the exact ESLint
`pathGroups` configuration so the generator behavior is unambiguous: state the
`alphabetize: { order: 'asc' }`, `newlines-between: 'always'`, the `groups`
order (`builtin`, `external`, `internal`, `['parent','sibling','index']`), and
provide the exact `pathGroups` entries for `cozy-*` and `twake-*` plus the
`distinctGroup` setting; place these identifiers (`pathGroups`, `alphabetize`,
`newlines-between`, `groups`, `distinctGroup`, `cozy-*`, `twake-*`) inline in
the doc so implementers can copy them verbatim into ESLint config without
interpretation drift.
In `@packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInput.jsx`:
- Around line 124-142: Add the missing showRemove and onRemove props to
FieldInput's contract: update FieldInput.propTypes to include showRemove:
PropTypes.bool and onRemove: PropTypes.func, and add default values in
FieldInput.defaultProps (e.g., showRemove: false and onRemove: null) so the
component declares the new optional boolean flag and the optional callback
handler; modify the existing FieldInput.propTypes and FieldInput.defaultProps
blocks accordingly.
In
`@packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInputWrapper.jsx`:
- Line 41: FieldInputWrapper currently forces size="small" which overrides any
size passed from parent props; update FieldInputWrapper.jsx (the
FieldInputWrapper component) to stop hard-coding size and instead respect an
incoming size prop (e.g., use the passed-in size if present, otherwise default
to "small"), or spread {...props} so per-field size controls are preserved.
In `@packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/helpers.spec.js`:
- Around line 484-505: Add a regression test in the ContactForm helpers.spec.js
that covers duplicate custom field names with conflicting entries: call
makeFields with defaultFields and a customFields array containing two entries
for the same name (one entry having isRemoved: true and another having position
or other overrides), ensure the resulting array does not include that field
(i.e., the isRemoved entry takes precedence regardless of order), and add
assertions for both input orders (removed-first and removed-second) to guarantee
deterministic behavior of makeFields.
In `@packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/index.jsx`:
- Around line 97-100: The map in ContactForm rendering uses the array index as
the key for each Box (the _fields.map callback); replace the unstable key with a
stable unique identifier from each field object (e.g., attributes.id,
attributes.key, or attributes.name) so React can correctly preserve component
state when field order changes—update the key prop on the Box in the _fields.map
to use that stable identifier (or generate/store a persistent id when fields are
created) instead of index.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02b3c2a1-0b32-454a-af25-f25a514a4127
📒 Files selected for processing (11)
AGENTS.mdpackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInput.jsxpackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInputArray.jsxpackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInputLayout.jsxpackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInputWrapper.jsxpackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/RemoveButton.jsxpackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/fieldsConfig.jsxpackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/helpers.jspackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/helpers.spec.jspackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/index.jsxpackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/styles.styl
💤 Files with no reviewable changes (1)
- packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/styles.styl
| const RemoveButton = ({ onRemove }) => { | ||
| return ( | ||
| <ListItemIcon className="u-ml-half"> | ||
| <IconButton aria-label="delete" size="medium" onClick={onRemove}> |
There was a problem hiding this comment.
Localize the delete button accessibility label.
Line 11 uses a hardcoded English aria-label, which makes screen-reader text non-localized in an otherwise translated form flow.
Suggested change
-const RemoveButton = ({ onRemove }) => {
+const RemoveButton = ({ onRemove, ariaLabel = 'delete' }) => {
return (
<ListItemIcon className="u-ml-half">
- <IconButton aria-label="delete" size="medium" onClick={onRemove}>
+ <IconButton aria-label={ariaLabel} size="medium" onClick={onRemove}>
<Icon icon={CrossCircleIcon} />
</IconButton>
</ListItemIcon>
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <IconButton aria-label="delete" size="medium" onClick={onRemove}> | |
| const RemoveButton = ({ onRemove, ariaLabel = 'delete' }) => { | |
| return ( | |
| <ListItemIcon className="u-ml-half"> | |
| <IconButton aria-label={ariaLabel} size="medium" onClick={onRemove}> | |
| <Icon icon={CrossCircleIcon} /> | |
| </IconButton> | |
| </ListItemIcon> | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/RemoveButton.jsx` at
line 11, The IconButton in RemoveButton.jsx currently uses a hardcoded English
aria-label ("delete"); replace this with a localized string obtained from the
app's i18n system (e.g. use the existing translation hook or helper in scope)
and pass that value to aria-label so screen readers receive the translated text;
locate the IconButton component in RemoveButton.jsx (the element with
onClick={onRemove}) and swap the literal aria-label for the translated key (e.g.
t('contacts.delete') or the appropriate namespace) ensuring fallback/defaults
are preserved.
to show 2 fields on the same row
zatteo
reviewed
Apr 2, 2026
| name={name} | ||
| contact={contact} | ||
| component={FieldInputWrapper} | ||
| showRemove={showRemove} |
Member
There was a problem hiding this comment.
Maybe you should do showRemove={showRemove && isMobile} instead of managing mobile directly inside the component?
It is managed differently between desktop and mobile.
Contributor
Author
There was a problem hiding this comment.
it's a good point 🤔 I prefer to separate responsibilities. In that case doing showRemove={showRemove && isMobile} will have same result, yes, but responsibilities are not the same. I think it will be easier that way to refactor in the future, if necessary
Member
There was a problem hiding this comment.
Responsabilities are better separated if we move it to the parent app now? I feel that the FieldAndRemove does "too much". But non blocking for me.
Contributor
Author
There was a problem hiding this comment.
Because we have the same behavior in 2 apps right now (admin & contacts).
There was a problem hiding this comment.
♻️ Duplicate comments (2)
AGENTS.md (2)
37-37:⚠️ Potential issue | 🟡 MinorFix wildcard notation in the import pattern line.
The pattern still says
cozy-_/twake-_; this is ambiguous and should usecozy-*/twake-*to match package names correctly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` at line 37, Update the import pattern line in AGENTS.md to use proper glob/wildcard notation by replacing the ambiguous `cozy-_` and `twake-_` substrings with `cozy-*` and `twake-*` so the pattern correctly matches package names (i.e., change the occurrences in the import pattern that currently read `cozy-_` / `twake-_` to `cozy-*` / `twake-*`).
53-73:⚠️ Potential issue | 🟡 MinorMake the example consistent with the rules it documents.
This snippet still uses double quotes and semicolons, which contradicts the stated conventions (“single quotes” and “no semicolons”).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 53 - 73, The example uses double quotes and semicolons which contradict the documented conventions; update the MyComponent example to use single quotes everywhere and remove all semicolons (imports, JSX attributes, and export) so FieldInput, makeIsRequiredError and MyComponent references remain unchanged while the file follows the "single quotes" and "no semicolons" style rules.
🧹 Nitpick comments (1)
packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/index.jsx (1)
97-100: Use field name as stable key instead of array index to prevent incorrect DOM reuse when fields are reordered or removed.The
_fieldsarray passed frommakeFields()can have its composition and order modified (viapositionproperty for reordering andisRemovedflag for removal), making index-based keys unstable. Each field object in the config has a guaranteednameproperty that persists across changes.Suggested change
- {_fields.map((attributes, index) => ( + {_fields.map((attributes, index) => ( <Box - key={index} + key={attributes.name || index} width={🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/index.jsx` around lines 97 - 100, The map over _fields in ContactForm currently uses the array index as the React key, which is unstable when makeFields() can reorder or remove fields; change the key on the <Box> rendered inside _fields.map to use the stable field identifier attributes.name (i.e., key={attributes.name}) instead of key={index}; if you want a defensive fallback, combine attributes.name with index (e.g., `${attributes.name}-${index}`) but prefer attributes.name alone since name is guaranteed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@AGENTS.md`:
- Line 37: Update the import pattern line in AGENTS.md to use proper
glob/wildcard notation by replacing the ambiguous `cozy-_` and `twake-_`
substrings with `cozy-*` and `twake-*` so the pattern correctly matches package
names (i.e., change the occurrences in the import pattern that currently read
`cozy-_` / `twake-_` to `cozy-*` / `twake-*`).
- Around line 53-73: The example uses double quotes and semicolons which
contradict the documented conventions; update the MyComponent example to use
single quotes everywhere and remove all semicolons (imports, JSX attributes, and
export) so FieldInput, makeIsRequiredError and MyComponent references remain
unchanged while the file follows the "single quotes" and "no semicolons" style
rules.
---
Nitpick comments:
In `@packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/index.jsx`:
- Around line 97-100: The map over _fields in ContactForm currently uses the
array index as the React key, which is unstable when makeFields() can reorder or
remove fields; change the key on the <Box> rendered inside _fields.map to use
the stable field identifier attributes.name (i.e., key={attributes.name})
instead of key={index}; if you want a defensive fallback, combine
attributes.name with index (e.g., `${attributes.name}-${index}`) but prefer
attributes.name alone since name is guaranteed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aa122781-e985-4858-b48c-ce7e36302369
📒 Files selected for processing (11)
AGENTS.mdpackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInput.jsxpackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInputArray.jsxpackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInputLayout.jsxpackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInputWrapper.jsxpackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/RemoveButton.jsxpackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/fieldsConfig.jsxpackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/helpers.jspackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/helpers.spec.jspackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/index.jsxpackages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/styles.styl
💤 Files with no reviewable changes (1)
- packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/styles.styl
✅ Files skipped from review due to trivial changes (2)
- packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/RemoveButton.jsx
- packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInputArray.jsx
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInputLayout.jsx
- packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/helpers.spec.js
- packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/fieldsConfig.jsx
- packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/helpers.js
- packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInput.jsx
- packages/cozy-ui-plus/src/Contacts/AddModal/ContactForm/FieldInputWrapper.jsx
zatteo
reviewed
Apr 2, 2026
zatteo
approved these changes
Apr 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests