Introduce <ArrayInputBase>, <SimpleFomIteratorBase> and <SimpleFormIteratorItemBase>
#10955
Conversation
…ormIteratorItemBase>`
| }); | ||
|
|
||
| return ( | ||
| <SimpleFormIteratorBase {...props} onAddItem={handleAddItem}> |
There was a problem hiding this comment.
| <SimpleFormIteratorBase {...props} onAddItem={handleAddItem}> | |
| <SimpleFormIteratorBase {...props} getItemDefaults={getItemDefaults}> |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces three new base components (<ArrayInputBase>, <SimpleFormIteratorBase>, and <SimpleFormIteratorItemBase>) to facilitate code reuse when implementing custom UI libraries. These components extract the core logic from existing UI-specific components to ra-core, enabling developers to build new UI implementations without duplicating business logic.
- Introduces new base components in
ra-corefor ArrayInput and SimpleFormIterator functionality - Refactors existing Material-UI components to use the new base components
- Adds comprehensive test-ui components for testing the new functionality
Reviewed Changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ra-core/src/controller/input/ArrayInputBase.tsx | New base component for array input functionality |
| packages/ra-core/src/controller/input/SimpleFormIteratorBase.tsx | New base component for form iterator logic |
| packages/ra-core/src/controller/input/SimpleFormIteratorItemBase.tsx | New base component for iterator item functionality |
| packages/ra-ui-materialui/src/input/ArrayInput/ArrayInput.tsx | Refactored to use ArrayInputBase |
| packages/ra-ui-materialui/src/input/ArrayInput/SimpleFormIterator.tsx | Refactored to use SimpleFormIteratorBase |
| packages/ra-ui-materialui/src/input/ArrayInput/SimpleFormIteratorItem.tsx | Refactored to use SimpleFormIteratorItemBase |
| packages/ra-core/src/test-ui/* | New test UI components for testing base components |
| docs_headless/src/content/docs/ArrayInputBase.md | Documentation for new ArrayInputBase component |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| export * from './SimpleFormIterator'; | ||
| export * from './SimpleShowLayout'; | ||
| export * from './TextInput'; | ||
| export * from './TextInput'; |
There was a problem hiding this comment.
Duplicate export of './TextInput' on lines 8 and 9. Remove one of these duplicate exports.
| export * from './TextInput'; |
Copilot uses AI. Check for mistakes.
|
|
||
| return ( | ||
| <div> | ||
| {} |
There was a problem hiding this comment.
Empty object literal serves no purpose and should be removed.
| {} |
Copilot uses AI. Check for mistakes.
| // @deprecated use useSimpleFormIteratorItem().remove instead | ||
| onRemoveField?: (index: number) => void; | ||
| // @deprecated use useSimpleFormIteratorItem().reOrder instead |
There was a problem hiding this comment.
Missing space in deprecation comment. Should be '// @deprecated Use useSimpleFormIteratorItem().remove instead' and '// @deprecated Use useSimpleFormIteratorItem().reOrder instead'.
| // @deprecated use useSimpleFormIteratorItem().remove instead | |
| onRemoveField?: (index: number) => void; | |
| // @deprecated use useSimpleFormIteratorItem().reOrder instead | |
| // @deprecated Use useSimpleFormIteratorItem().remove instead | |
| onRemoveField?: (index: number) => void; | |
| // @deprecated Use useSimpleFormIteratorItem().reOrder instead |
Copilot uses AI. Check for mistakes.
| <div>Tags:</div> | ||
| <ArrayInputBase source="tags"> |
There was a problem hiding this comment.
| <div>Tags:</div> | |
| <ArrayInputBase source="tags"> | |
| <div>Items:</div> | |
| <ArrayInputBase source="items"> |
| ) | ||
| ``` | ||
|
|
||
| **Note**: Setting [`shouldUnregister`](https://react-hook-form.com/docs/useform#shouldUnregister) on a form should be avoided when using `<ArrayInputBase>` (which internally uses `useFieldArray`) as the unregister function gets called after input unmount/remount and reorder. This limitation is mentioned in the react-hook-form [documentation](https://react-hook-form.com/docs/usecontroller#props). If you are in such a situation, you can use the [`transform`](./EditBase.html#transform) prop to manually clean the submitted values. |
There was a problem hiding this comment.
| **Note**: Setting [`shouldUnregister`](https://react-hook-form.com/docs/useform#shouldUnregister) on a form should be avoided when using `<ArrayInputBase>` (which internally uses `useFieldArray`) as the unregister function gets called after input unmount/remount and reorder. This limitation is mentioned in the react-hook-form [documentation](https://react-hook-form.com/docs/usecontroller#props). If you are in such a situation, you can use the [`transform`](./EditBase.html#transform) prop to manually clean the submitted values. | |
| **Note**: Setting [`shouldUnregister`](https://react-hook-form.com/docs/useform#shouldUnregister) on a form should be avoided when using `<ArrayInputBase>` (which internally uses `useFieldArray`) as the unregister function gets called after input unmount/remount and reorder. This limitation is mentioned in the react-hook-form [documentation](https://react-hook-form.com/docs/usecontroller#props). If you are in such a situation, you can use the [`transform`](./EditBase.md#transform) prop to manually clean the submitted values. |
| | Prop | Required | Type | Default | Description | | ||
| |-----------------| -------- |---------------------------| ------- |---------------------------------------------------------------------------------------------------------------------------------------------------------------------| | ||
| | `source` | Required | `string` | - | Name of the entity property to use for the input value | | ||
| | `defaultValue` | Optional | `any` | - | Default value of the input. | | ||
| | `readOnly` | Optional | `boolean` | `false` | If true, the input is in read-only mode. | | ||
| | `disabled` | Optional | `boolean` | `false` | If true, the input is disabled. | | ||
| | `validate` | Optional | `Function` | `array` | - | Validation rules for the current property. See the [Validation Documentation](./Validation.md#per-input-validation-built-in-field-validators) for details. | No newline at end of file |
There was a problem hiding this comment.
isPending and loading are not documented. Is this intended?
There was a problem hiding this comment.
That actually makes me wonder whether they should be there at all
| --- | ||
| layout: default | ||
| title: "<ArrayInputBase>" | ||
| --- |
There was a problem hiding this comment.
You forgot to add this file (and the other one too) to the sidebar navigation.
| {fields.map((member, index) => ( | ||
| <SimpleFormIteratorItemBase | ||
| key={member.id} | ||
| fields={fields} |
There was a problem hiding this comment.
If my understanding is correct, passing fields here is no longer necessary since the getItemDefaults prop was introduced. Am I correct?
| export const useGetArrayInputNewItemDefaults = ( | ||
| fields: ArrayInputContextValue['fields'] | ||
| ) => { |
There was a problem hiding this comment.
Did you check if this code is still necessary with latest versions of RHF?
Actually I did test adding a defaultValue on a input in the story in ra-core (i.e. which does not leverage this helper), and the default value was applied alright, which tends to indicate this whole function may no longer be necessary.
Worth checking out IMHO.
PS: this could be an interesting story to add btw
There was a problem hiding this comment.
It does not really work. The should not reapply default values set at form level after removing and then re-adding one row, even with FormDataConsumer test fails because RHF restore the value of the removed item when you add a new one.
So yes, I'm afraid this code is still necessary.
| <SimpleFormIteratorClearButton | ||
| disableClear={disableClear} | ||
| disableRemove={disableRemove} | ||
| /> |
There was a problem hiding this comment.
You did not report the SimpleFormIteratorClasses.clear className. Is that intended? 🤔
| remove: () => remove(index), | ||
| }), | ||
| [index, total, reOrder, remove] | ||
| const record = useRecordContext(); |
There was a problem hiding this comment.
| const record = useRecordContext(); | |
| const record = useRecordContext(props); |
Otherwise it's a BC
Problem
When implementing new UI (e.g. ShadnAdminKit), we have to repeat a lot of code that could be in
ra-core.Solution
Introduce
<ArrayInputBase>,<SimpleFomIteratorBase>and<SimpleFormIteratorItemBase>How To Test
Additional Checks
masterfor a bugfix or a documentation fix, ornextfor a featureAlso, please make sure to read the contributing guidelines.