Skip to content
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

Adding an item to ArrayInput on Edit after removing one brings back the same item that was removed #7385

Closed
dejancg opened this issue Mar 12, 2022 · 10 comments · Fixed by #8029
Labels

Comments

@dejancg
Copy link

dejancg commented Mar 12, 2022

What you were expecting:
Removing an item from ArrayInput should really remove it.

What happened instead:
After removing an item and adding a new one, the previosuly removed item is back.

Steps to reproduce:
Start here: codesandbox.io/s/happy-germain-877ypu

  1. Create a new post using the floating + button in bottom right,, fill in the required fields and make sure it has a backlink added. Click on Save and edit.
  2. When Edit view has opened, click on "Miscellaneous" tab to edit backlinks.
  3. Remove the backlink.
  4. Add a backlink.
  5. Observe, the backlink you previosly removed is now back.
    I have added a video for clarification:
2022-03-14_12-42-48.mp4

Other information:
There is a workaround which fixed the problem for me:

const AddItemButton = ({ onClick, ...rest }: ButtonProps) => {
  const { change, getFieldState } = useForm();
  return (
    <Button
      label="ra.action.add"
      {...rest}
      onClick={(e) => {
        const fieldState = getFieldState("formArrayFieldSource");
        const nextIndex = fieldState?.value && Array.isArray(fieldState.value) ? fieldState.value.length : 0;
        onClick && onClick(e);
        change(`formArrayFieldSource[${nextIndex}]`, {
          ...getEmptyArrayItem()
        });
      }}>
      <AddCircleOutlineRoundedIcon />
    </Button>
  );
};

where getEmptyArrayItem is a function which returns an empty array item, such that it doesn't have any undefined members.
The idea behind it is to manually make sure the newly added array item is really new.

Environment

  • React-admin version: 3.19.10 (tried with 3.19.3 too but locally, same problem)
  • Last version that did not exhibit the issue (if applicable): N/A
  • React version: 16 locally, but the sandbox uses 17
  • Browser: Chrome latest
@djhi
Copy link
Contributor

djhi commented Mar 14, 2022

Reproduced. Thanks for the report

@djhi djhi added the bug label Mar 14, 2022
@djhi
Copy link
Contributor

djhi commented Mar 14, 2022

The codesandbox is actually using the beta and the latest react and MUI versions. I can't reproduce the issue on the latest v3, only on the v4 beta.

@djhi djhi added this to the 4.0.0-beta.3 milestone Mar 14, 2022
@dejancg
Copy link
Author

dejancg commented Mar 14, 2022

Hey @djhi, I have forked the sandbox: https://codesandbox.io/s/happy-germain-877ypu
I am pretty sure it uses react-admin v3.19.10:

Screenshot

@djhi
Copy link
Contributor

djhi commented Mar 14, 2022

Nope, you can see the version actually used by hovering each line

@dejancg
Copy link
Author

dejancg commented Mar 14, 2022

That's an awful UX then, but here, try again, I have made sure v3 is used and I can reproduce this: https://codesandbox.io/s/happy-germain-877ypu

Make sure you save the post first and go to edit when testing.

Because I needed a workaround for this which doesn't involve spending too much time on it, I was able to come up with this:

const AddItemButton = ({ onClick, ...rest }: ButtonProps) => {
  const { change, getFieldState } = useForm();
  return (
    <Button
      label="ra.action.add"
      {...rest}
      onClick={(e) => {
        const fieldState = getFieldState("formArrayFieldSource");
        const nextIndex = fieldState?.value && Array.isArray(fieldState.value) ? fieldState.value.length : 0;
        onClick && onClick(e);
        change(`formArrayFieldSource[${nextIndex}]`, {
          ...getEmptyArrayItem()
        });
      }}>
      <AddCircleOutlineRoundedIcon />
    </Button>
  );
};

where getEmptyArrayItem is a function which returns an empty array item, such that it doesn't have any undefined members.
The idea behind it is to manually make sure the newly added array item is new

@djhi
Copy link
Contributor

djhi commented Mar 14, 2022

Agreed, this part of their UI is weird. I can't reproduce the issue on your codesandbox though

@dejancg
Copy link
Author

dejancg commented Mar 14, 2022

Not sure why you can't reproduce it, I tried on two different computers and I was able to. I have updated the issue with the video and workaround which did it for me.

@fzaninotto fzaninotto removed this from the 4.0.0-beta.3 milestone Mar 16, 2022
This was referenced Mar 23, 2022
@mpaciu
Copy link

mpaciu commented Apr 18, 2022

Hi! I have this same problem in v3.19.10 version like @dejancg. @djhi tested this issue but maybe need the condition of some material-ui version. Could @djhi tell me which versión of material did you use? The issue happens in my case with this context:

"dependencies": {
"@material-ui/core": "^4.12.3",
"@material-ui/icons": "^4.11.2",
"@mui/icons-material": "^5.5.1",
"@mui/material": "^5.5.2",
"react": "^16.14.0",
"react-admin": "^3.19.10",
"react-dom": "^16.14.0",
"react-scripts": "^2.1.8",
}

I traied to change version in the codesanbox but the project crash, i haven´t so much experience with this tool.

Also i would to know if in the last versión 4.0.1 is solved.

Thanks you so much!

@slax57
Copy link
Contributor

slax57 commented Apr 20, 2022

I tried running the project locally to be sure it's not a problem caused by codesandbox.
I reproduced the issue both with the latest v3 (react-admin@3.19.11 and @material-ui/core@4.12.3) and also with v4 (react-admin@4.0.0).

@djhi
Copy link
Contributor

djhi commented Aug 2, 2022

So this happens because we don't provide the values for the new item as explained in react-hook-form documentation.

I can only think of two solutions for this, probably complementary:

  • We add a newItemData prop to the ArrayInput that will be passed to the append function
  • We inspect children to get their defaultValue prop. I don't like this as we try to avoid inspecting children since v4 as it has proven to often be problematic.

Open to suggestions.

djhi pushed a commit that referenced this issue Aug 3, 2022
Seojun-Park pushed a commit to Seojun-Park/react-admin that referenced this issue Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants