Feat: `SimpleFormIterator` clear item call action #8302

Seojun-Park · 2022-10-25T12:56:55Z

This PR is related to this issue : #8287

Screen.Recording.2022-10-25.at.12.21.05.mov

WiXSL · 2022-10-25T13:29:06Z

This looks really good!
It is necessary for us that you add some unit tests and at least a story to showcase the new feature
Plus, since is a new feature, you have to target it from next branch instead of master

Seojun-Park · 2022-10-25T15:45:14Z

Target is changed to next and story test is added

Thanks!

WiXSL · 2022-10-25T16:43:25Z

Could you rebase?

Seojun-Park · 2022-10-25T16:51:01Z

@WiXSL

I'm asking this just to be sure :)
do you want me to rebase to next branch?

WiXSL · 2022-10-25T16:53:34Z

Sure, right now, your history shows changes in 61 files. It is difficult to follow

WiXSL · 2022-10-25T18:39:41Z

You've added a story for storybook, but you need to add unit tests as well and documentation

fzaninotto · 2022-10-26T12:59:15Z

Hi, and thanks for your PR.

I'm not fond of the UI you're proposing. The "refresh" icon is misleading in this case, and the icon color should be the same as the 'remove item' icon since it's a destructive action.

fzaninotto · 2022-11-23T09:12:49Z

docs/SimpleFormIterator.md

@@ -70,6 +70,7 @@ const OrderEdit = () => (
 | Prop     | Required | Type           | Default               | Description                         |
 |----------|----------|----------------|-----------------------|-------------------------------------|
 | `addButton` | Optional | `ReactElement` | - | Component to render for the add button |
+| `clearButton` | Optional | `boolean` | `true` | When `false`, the user cannot clear the array |


If it's a simple boolean, then the prop name isn't right.

In general, we prefer to define boolean props that take the value true when used, so:

<SimpleFormIterator disableClear>

is preferable to:

<SimpleFormIterator clearButton={false}>

In addition, we already have other disableXXX props for this component, so for consistency this prop should follow the same pattern.

updated : b987e62

fzaninotto · 2022-11-23T09:13:08Z

docs/SimpleFormIterator.md

@@ -94,6 +95,10 @@ This prop lets you pass a custom element to replace the default Add button.
 </SimpleFormIterator>
 ```

+## `clearButton`
+
+This prop lets you pass a custom element to replace the default "Clear the list" button


That's not true anymore

Removed and disableClear description is added : b987e62

fzaninotto

Almost there!

fzaninotto · 2022-11-23T09:41:29Z

packages/ra-ui-materialui/src/input/ArrayInput/SimpleFormIterator.tsx

-                    </div>
-                )}
+                <div className={SimpleFormIteratorClasses.buttons}>
+                    {!disabled && !disableAdd && (


In the DisableAdd story, I see that this also disables the ClearButton...

Indeed, disableAdd condition should not effect to disableClear

resolve commit : 2f21d4d

fzaninotto · 2022-11-23T09:53:15Z

packages/ra-ui-materialui/src/input/ArrayInput/SimpleFormIterator.tsx

-                    </div>
-                )}
+                <div className={SimpleFormIteratorClasses.buttons}>
+                    {!disabled && (


But then if both disableAdd and disableRemove are true, we will insert an empty div for nothing...

empty div should not render, I agree

resolve commit : 2646756

While doing this, I realized that array clear button has add suffix on classname. I guess the each button's class name should have relate to the its behavior.
I added poc commit for verification : dd187c5

fzaninotto · 2022-11-27T21:01:44Z

Thanks

Seojun-Park marked this pull request as ready for review October 25, 2022 12:58

vercel bot deployed to Preview – react-admin-storybook October 25, 2022 12:58 View deployment

Seojun-Park changed the base branch from master to next October 25, 2022 15:44

vercel bot deployed to Preview – react-admin-storybook October 25, 2022 15:54 View deployment

Seojun-Park force-pushed the array-clear-button branch from 22bfc7c to a842231 Compare October 25, 2022 16:43

vercel bot deployed to Preview – react-admin-storybook October 25, 2022 16:45 View deployment

Seojun-Park force-pushed the array-clear-button branch from a842231 to f01152d Compare October 25, 2022 17:25

vercel bot deployed to Preview – react-admin-storybook October 25, 2022 17:27 View deployment

Seojun-Park force-pushed the array-clear-button branch from f01152d to a842231 Compare October 25, 2022 17:47

soullivaneuh force-pushed the array-clear-button branch from a842231 to 27c093e Compare October 25, 2022 17:53

vercel bot deployed to Preview – react-admin-storybook October 25, 2022 17:55 View deployment

WiXSL added the RFR Ready For Review label Oct 25, 2022

Seojun-Park force-pushed the array-clear-button branch 10 times, most recently from 27c093e to 40ed981 Compare October 26, 2022 14:18

vercel bot deployed to Preview – react-admin-storybook October 26, 2022 14:19 View deployment

Seojun-Park requested review from soullivaneuh, WiXSL and fzaninotto and removed request for WiXSL, soullivaneuh and fzaninotto November 23, 2022 09:06

fzaninotto requested changes Nov 23, 2022

View reviewed changes

update disable-clear

b987e62

vercel bot deployed to Preview – react-admin-storybook November 23, 2022 09:37 View deployment

fzaninotto requested changes Nov 23, 2022

View reviewed changes

move disable-add condition not to effect array clear

2f21d4d

vercel bot deployed to Preview – react-admin-storybook November 23, 2022 09:51 View deployment

fzaninotto requested changes Nov 23, 2022

View reviewed changes

add condition not to render div

2646756

vercel bot deployed to Preview – react-admin-storybook November 23, 2022 10:19 View deployment

poc: split buttons with proper class name

dd187c5

vercel bot deployed to Preview – react-admin-storybook November 23, 2022 10:27 View deployment

Seojun-Park requested review from fzaninotto and removed request for WiXSL November 24, 2022 09:06

fzaninotto approved these changes Nov 27, 2022

View reviewed changes

fzaninotto added this to the 4.6 milestone Nov 27, 2022

fzaninotto mentioned this pull request Nov 27, 2022

SimpleFormIterator: Clear items call to action #8287

Closed

fzaninotto merged commit 4487b6e into marmelab:next Nov 27, 2022

Seojun-Park deleted the array-clear-button branch November 28, 2022 13:02

slax57 mentioned this pull request Dec 5, 2022

[Doc] Update <ArrayInput> preview with clear all button #8467

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: `SimpleFormIterator` clear item call action #8302

Feat: `SimpleFormIterator` clear item call action #8302

Seojun-Park commented Oct 25, 2022 •

edited

Loading

WiXSL commented Oct 25, 2022 •

edited

Loading

Seojun-Park commented Oct 25, 2022 •

edited

Loading

WiXSL commented Oct 25, 2022

Seojun-Park commented Oct 25, 2022

WiXSL commented Oct 25, 2022 •

edited

Loading

WiXSL commented Oct 25, 2022 •

edited

Loading

fzaninotto commented Oct 26, 2022

fzaninotto Nov 23, 2022

Seojun-Park Nov 23, 2022

fzaninotto Nov 23, 2022

Seojun-Park Nov 23, 2022

fzaninotto left a comment

fzaninotto Nov 23, 2022

Seojun-Park Nov 23, 2022

fzaninotto Nov 23, 2022

Seojun-Park Nov 23, 2022

fzaninotto commented Nov 27, 2022

Feat: SimpleFormIterator clear item call action #8302

Feat: SimpleFormIterator clear item call action #8302

Conversation

Seojun-Park commented Oct 25, 2022 • edited Loading

WiXSL commented Oct 25, 2022 • edited Loading

Seojun-Park commented Oct 25, 2022 • edited Loading

WiXSL commented Oct 25, 2022

Seojun-Park commented Oct 25, 2022

WiXSL commented Oct 25, 2022 • edited Loading

WiXSL commented Oct 25, 2022 • edited Loading

fzaninotto commented Oct 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fzaninotto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fzaninotto commented Nov 27, 2022

Feat: `SimpleFormIterator` clear item call action #8302

Feat: `SimpleFormIterator` clear item call action #8302

Seojun-Park commented Oct 25, 2022 •

edited

Loading

WiXSL commented Oct 25, 2022 •

edited

Loading

Seojun-Park commented Oct 25, 2022 •

edited

Loading

WiXSL commented Oct 25, 2022 •

edited

Loading

WiXSL commented Oct 25, 2022 •

edited

Loading