React-admin: "Undo" feature is misleading and should be renamed or re-implemented

[normanzb]

Is your feature request related to a problem? Please describe.
The feature "undoable" is misleading.

Calling it "undo" makes data operator thinking the data is already committed to database at the time they click the button, so they are confused into thinking it is safe to check the data on the actual webpage that uses it. It often make them come back to devs asking why the data wasn't showing, then the dev checks the webpage, it does shows the data, as it took 5 seconds for the request to actually be sent. This makes them even more confused and worrying if they took too much drug last night and had some long lasting hallucination.

Combined with Ticket https://github.com/marmelab/react-admin/issues/1857 and https://github.com/marmelab/react-admin/issues/2721, they make the situation worse.

Also because of https://github.com/marmelab/react-admin/issues/2425, undoable can't be turned off in my app.

This feature really has nothing todo with "undo", the data hasn't been sent to server at the time user click the edit button. So if they hasn't done anything yet, how can they undo it?

Describe the solution you'd like
There are 3 possible solutions:

1. Implement the real undoable: IMO the true "undoable" should commit the changes immediately to server and keep a record of every changes, when "undo" is clicked, the record in client memory is then used to recover the original version of data.
2. Rename the feature, I think this feature should be called something like "regrettable action" or "recall", just like what is in Outlook to recall a regrettable e-mail.
3. I think the UI should also be updated to show a count down message: "The missile is launching in 5, 4, 3... cancel it now before it is too late!"

[fzaninotto]

Hi, and thanks for your suggestion. Indeed, the undo feature may be misleading. Regarding the solutinos you suggest:

1. we can't assume that the API supports undoing actions - very few APIs do. So relying on the API for undo is a no go
2. I'd gladly rename it, but none of your suggestions are convincing. The label is usually "Element updated" or "Element deleted", what better message would you propose?
3. adding a countdown is distracting IMO, especially if it happens on every edit / delete. I'm -1 for this solution.

I think the right solution is to make it easy for the developer to disable undo for a resource if the users find it disturbing.

[normanzb]

Thanks for reply @fzaninotto , regarding to the 2nd question, how about "Element update is queued - Cancel"

+1 to allow develop to disable undo but #1857 and #2721 makes it difficult

[djhi]

It's already possible but not documented. The Edit and DeleteButton components accept an undoable boolean prop which is true by default.

[michanismus]

Same here - represents not even close an "undo" functionality. And activated by default is very annoying...
You should rename it to "fingers crossed delayed API call feature".

[djhi]

If you dont like the text, use your own translations instead of the default

[normanzb]

thanks @djhi , that's good idea, I will try to change the translation for our admin tool
but with all due respect in my opinion this is still quite confusing for any other developers or their future users if left as default setting.

I am closing this ticket now as it doesn't gain consensus, and the problem on my end can be solved by changing the translation

Please reopen you think it worths it.

[JohnWeisz]

I'd like to apologize about my rant in advance, but unfortunately, I also feel "Undo" was implemented in an extremely piss-poor fashion here:

• First and foremost, it's not an "undo", it doesn't undo anything, it essentially only delays/cancels the request from being sent through the data-provider.
• At best, this allows accidental clicks on Delete to be _canceled_ for a few brief seconds if you are quick (unless you accidentally click somewhere else, or your mouse button is already down, because then you just nuked your data without confirmation).
• Using a snackbar here is probably the worst possible choice: snackbars require that the action is accessible later, and users have came to learn that not acting on the snackbar action is not a problem, as it can be done later in some other way. Here, it implies that there is some sort of an undo-history, but that's not the case.

Please either fix it properly or disable it by default, because currently, it's literally destructive.

[djhi]

Well, first the vocabulary some you used to speak to us is unacceptable (extremely piss-poor fashion, for example) and I'm actually very tempted to simply ignore you.

Second, we won't make it harder for more than 7000 people who starred the repo to activate the feature because a very little few people think it's wrong.

However, as you have the right to disagree with us, you can simply define your own defaults in your projects or your fork of this library. For example:

import { Edit as ReactAdminEdit } from 'react-admin';

const Edit = ReactAdminEdit;

Edit.defaultProps = {
    undoable: false,
};

export default Edit;

You can then use this Edit everywhere in your project.

PS: inappropriate langage => banned from this repository. There will be no other warning

[normanzb]

Second, we won't make it harder for more than 7000 people who starred the repo to activate the feature because a very little few people think it's wrong.

This isn't a very fair comparison, this ticket hasn't got a lot of exposure to gain consensus. In my opinion if we can get this issue sorted, 7000 stars won't go away and we will see less complaining here.

[JohnWeisz]

@djhi
I believe you've misunderstood my intentions or my general view about this project, please accept my apologies about _that_ (I haven't used an expression that I would personally find insulting, unacceptable, or inappropriate, and that wasn't the point in the first place, so no need to go personal there, I believe this is merely a misunderstanding).

The problem IMO, that I wanted to stress, is that the current implementation of Undo is "dangerously" incomplete, as it is not only misleading, but it's also tied to an UI action that's not accessible beyond a few brief seconds, while it should be a "permanently" available option (however that is defined in the context of an app) - this is a UX issue, but quite a big one due to the impact of the action being done.

Obviously, I understand that it's not quite possible to have a permanent Undo if it's not entirely supported through the backend.

[timotgl]

It took me a few hours of confused debugging to realize that the undo feature on the <Edit> view seems to break the dataProvider entirely if you redirect with an onSuccess prop callback. It does redirect immediately and sends no requests to the backend. However the route you're redirected to can't use the dataProvider anymore and the app is just kind of broken. It's not super obvious that only undoable={false} fixes this.

I feel like having undoable as an opt-in feature would be a better default. It adds complexity and has implications (as described in my case above). Also agree with @JohnWeisz points above about the benefits probably being overestimated.

Overall great software though, thanks for building it!

[knnnrd]

@djhi I'm also looking for a way to globably disable the "undoable" feature. I don't mind if it's on by default, as long as I can easily disable it. I'm using the snippet below to disable it for edit which is working fine.

import { Edit as ReactAdminEdit } from 'react-admin';

const Edit = ReactAdminEdit;

Edit.defaultProps = {
    undoable: false,
};

export default Edit;

However I'm struggling a bit to disable it in a reusable manner for the BulkDeleteButton. I want to be able to use my "global" List component which sets undoable to be disabled, and be able to pass extra components to bulkActionButtons I'm getting some console errors too for my test. I don't think I have to pass selectedIds manually? Nevertheless the export is working correctly..

Warning: Failed prop type: The prop `resource` is marked as required in `BulkExportButton`, but its value is `undefined`.
Warning: Failed prop type: The prop `selectedIds` is marked as required in `BulkExportButton`, but its value is `undefined`.

My code:

import { List as ReactAdminList, BulkDeleteButton } from 'react-admin';
import * as React from 'react';

const NonUndoableBulkActionButtons = props => (
    <React.Fragment>
      {props.buttons}
      <BulkDeleteButton {...props} undoable={false} />
    </React.Fragment>
);


export const List = props => (
  <ReactAdminList
    {...props}
    bulkActionButtons={<NonUndoableBulkActionButtons buttons={props.bulkActionButtons} />}
  />
)

import * as React from "react";
import { Datagrid, TextField, EditButton, ShowButton, BulkExportButton } from 'react-admin';
import { List } from '../List';     // See code above

const BulkActionButtons = props => (
  <React.Fragment>
    <BulkExportButton {...props} />
  </React.Fragment>
);


export const MyList = props => (
  <List {...props} bulkActionButtons={<BulkActionButtons label="foobarbaz" />}>
    <Datagrid>
      <TextField source={'abc'} />
      .......
      <ShowButton/>
      <EditButton/>
    </Datagrid>
  </List>
);

Is this the way to go? And I think that sooner or later I will probably bump into another component which needs to be wrapped like this. Thanks in advance.