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

customize delete button redirect action #3780

Closed
wants to merge 1 commit into from
Closed

Conversation

pxpm
Copy link
Contributor

@pxpm pxpm commented Jul 3, 2021

refs #3762

When deleting an entry from the show operation, although the entry is indeed deleted the browser stays in the entry show page. Refreshing the page would cause 404 error as the entry is already deleted from database.

This PR adds a fallback for when no crudTable is present on page and the entry is deleted we redirect to previous page.

@scrutinizer-notifier
Copy link

The inspection completed: No new issues

@tabacitu tabacitu removed the Bug label Jul 5, 2021
@tabacitu
Copy link
Member

tabacitu commented Jul 5, 2021

Hmm... I'm not sure I consider this to be a bug... From my perspective:

  • a success message is shown to the user, telling the item was deleted;
  • there's no reason for the person to refresh this page;
  • if the user DID refresh this page, I think it's the expected behaviour for it to fail; the item no longer exists;

Granted, what the user wants, most of the time, after deletion, is to do something else. So we could save them some time and just redirect them to the table view. But I wouldn't call this a bug to be honest...

I don't think we can implement a change like this and consider it non-breaking. That's because:

  • People might be loading these buttons in their custom operations, that don't have the crud js object. And they might not want this new behaviour.
  • I'm not sure the previous page is what they'd want; they probably want the table view; but then again, what if that doesn't exist?! tricky;

Because of the caveats above, I think it'd be better to just... not do anything 😀 I think sometimes it's difficult, but one should say "it's good enough" 😅 Please tell me if you think I'm wrong - it happens at least once a day 😅

Cheers!

@pxpm
Copy link
Contributor Author

pxpm commented Jul 9, 2021

Indeed, you are right about the implications. This was just a hacky solution that could work for one project.

But I still think deleting an entry and doing nothing is not the natural behaviour. Even in List, after we delete the entry, it gets deleted from table.

I will keep this around, because I think I have other PR opened to add a "different" redirect to the Cancel button. #3248

This might be inline with that I think it's needed for "Crud buttons". A way to define what happens when user click the button (go previous, go to specific page wtv).

@pxpm pxpm changed the title delete button redirect to previous page when there is no crud table customize delete button redirect action Jul 9, 2021
@pxpm pxpm closed this Aug 19, 2021
@pxpm pxpm deleted the delete-button-on-show branch August 19, 2021 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants