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

Unlinking a record gives a "Are you sure you want to delete this record?" message #1697

Open
2 tasks done
lekoala opened this issue Mar 1, 2024 · 1 comment
Open
2 tasks done

Comments

@lekoala
Copy link
Contributor

lekoala commented Mar 1, 2024

Module version(s) affected

5.1.x

Description

Currently, when you unlink a record, you get a warning message saying "Are you sure you want to delete this record?"

image

This is obviously NOT the case since you only UNLINK the record.

The toast message says "deleted" afterwards

How to reproduce

In model admin
Have a record with a has_many relation
Unlink a record
Get the message and see the toast

Possible Solution

This seems to be done on the js side of things

const confirmMessage = $(this).hasClass('action--archive')
? i18n._t('Admin.ARCHIVECONFIRMMESSAGE', 'Are you sure you want to archive this record?')
: i18n._t('Admin.DELETECONFIRMMESSAGE', 'Are you sure you want to delete this record?');

I would rather have some kind of data attribute that can be set on the button itself. In general, having "confirmable" actions handled at core level would be great, but that's another topic...

This way, the button can provide the message itself or even make it user configurable if needed. Much better than hardcoded in js.

Toasts message should rely on X-Status instead

Additional Context

No response

Validations

  • Check that there isn't already an issue that reports the same bug
  • Double check that your reproduction steps work in a fresh installation of silverstripe/installer (with any code examples you've provided)
@GuySartorelli
Copy link
Member

Thanks for reporting this.
If you'd like to raise a PR to resolve this I'd be very grateful. This almost certainly affects CMS 4 as well, so please target 1.13 if you do open a PR to resolve this.

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