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

disableRemove method on SimpleFormIterator does not work as expected #5553

Closed
etienne-bondot opened this issue Nov 19, 2020 · 11 comments
Closed

Comments

@etienne-bondot
Copy link

etienne-bondot commented Nov 19, 2020

What you were expecting:
The disableRemove is either a boolean or a function with the current record. That said, I want to be able to enable or disable the deleteButton on a specific record within a SimpleFormIterator.

This is from the RA code base https://github.com/marmelab/react-admin/blob/master/packages/ra-ui-materialui/src/form/SimpleFormIterator.tsx#L140

// Returns a boolean to indicate whether to disable the remove button for certain fields.
// If disableRemove is a function, then call the function with the current record to
// determining if the button should be disabled. Otherwise, use a boolean property that
// enables or disables the button for all of the fields.
const disableRemoveField = (record, disableRemove) => {
  if (typeof disableRemove === 'boolean') {
    return disableRemove;
  }
  return disableRemove && disableRemove(record);
}

What happened instead:
The record is either {} or undefined.

Steps to reproduce:
Create a SimpleFormIterator and add a disableRemove function.,

const handleDisableRemove = (record) => {
  console.log({record});
  return true;
};

<ArrayInput source="whatever">
  <SimpleFormIterator disableRemove={handleDisableRemove}>
  ...
  </SimpleFormIterator>
</ArrayInput>

Related code:
https://codesandbox.io/s/heuristic-violet-2modw?file=/src/posts/PostEdit.js

  • React-admin version: 3.10.1
  • React version: 16.14.0
@fzaninotto
Copy link
Member

Bug reproduced and confirmed, thanks for the report.

This seems to happen only when the field is empty.

@fzaninotto fzaninotto added the bug label Nov 19, 2020
@fzaninotto
Copy link
Member

fzaninotto commented Nov 19, 2020

Wait, it's not a bug, it's just that the documentation is wrong.

The record passed to the disableRemove function isn't the main record, but the current element iterated over in the array.

So for instance if the main object is:

{
    id: 1,
    title: 'Accusantium qui nihil voluptatum quia voluptas maxime ab similique',
    views: 143,
    published_at: new Date('2012-08-06'),
    backlinks: [
        {
            date: '2012-08-09T00:00:00.000Z',
            url: 'http://example.com/bar/baz.html',
        },
    ],
}

Then on an ArrayInput on the backlinks field:

<ArrayInput source="backlinks">
  <SimpleFormIterator disableRemove={handleDisableRemove}>
  ...
  </SimpleFormIterator>
</ArrayInput>

The handleDisableRemove function will be called with

{
            date: '2012-08-09T00:00:00.000Z',
            url: 'http://example.com/bar/baz.html',
}

And for new elements, it will be called with {}.

That's the feature as it was implemented in #2850. If you want to disable the remove button for all elements based on the actual record, that's an enhancement.

Would you like to work on a PR to enable that by passing the actual record as second argument to disableRemove?

@etienne-bondot
Copy link
Author

etienne-bondot commented Nov 19, 2020

I understand properly the documentation and I get that this is the current record and not the main record, which is not empty, that is the issue.

It should be called with

{
            date: '2012-08-09T00:00:00.000Z',
            url: 'http://example.com/bar/baz.html',
}

But it's not. I've gave you a reproducing example where you can see it live.

@fzaninotto
Copy link
Member

I can't reproduce the problem in the CodeSandbox you linked. When I edit a post with a non-empty backlinks (e.g. post #3), the console correctly shows the url and date:

image

@etienne-bondot
Copy link
Author

You right I see it again, don't know what happened on my side. Sorry for this time lost 😞.

@fzaninotto
Copy link
Member

So you don't need the remove button to vary according to the main record, right?

@etienne-bondot
Copy link
Author

etienne-bondot commented Nov 19, 2020

No I want to be able to disable the delete button according to the current item on iteration, but I should have something broken somewhere...

This what I get on my side, do you have any hint ?

const handleDisableRemove = (record) => {
    console.log({ record });
    return false;
};

<ArrayInput source="sessions">
  <SimpleFormIterator disableRemove={handleDisableRemove}>
    <FormDataConsumer>
      {({ getSource, scopedFormData }) => {
        console.log({ scopedFormData });
        return (.... some inputs)

and the output

scopedFormData: {
  id: "e9bdfd07-0da3-35bc-8c97-d4ca942c0622"
  isDefault: true
  languageEntries: {fr_FR: {…}, en_US: {…}}
  programType: "T&C"
  sessionDuration: 55
  sessionsNumber: 8
  type: "classic"
}

record:{}

I don't get it... 😔

@fzaninotto
Copy link
Member

well, if you can build a reproducible case based on the simple example, it means there is a bug on our side. If you can't, it means the bug is on your side...

@etienne-bondot
Copy link
Author

etienne-bondot commented Nov 19, 2020

@fzaninotto Actually this is working fine on Edit, but not on the Create view, try with https://codesandbox.io/s/heuristic-violet-2modw?file=/src/posts/PostCreate.js I updated it to reproduce the issue.

image

@etienne-bondot
Copy link
Author

Hi @fzaninotto, sorry to bother you but did you have time to check with the Create view of the sandbox ? I just want to know if this is a react-admin relative bug or just on my side. Thanks a lot!

@etienne-bondot
Copy link
Author

I am reopening this issue because my previous workaround does not fit the new case we have at work. The IT team at MoovOne agreed to contribute on open source projects and as we use RA for our admin we are happily going to investigate further on this bug within the Edit context.

scaperow added a commit to scaperow/react-admin that referenced this issue Jul 22, 2022
Solve the problem indirectly marmelab#5553
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants