WordPress.org

Make WordPress Core

Opened 7 months ago

Closed 3 months ago

Last modified 2 months ago

#39712 closed enhancement (fixed)

Improve wording of bulk delete warnings for media

Reported by: dllh Owned by: joemcgill
Milestone: 4.8 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch
Focuses: ui Cc:

Description

In #28235, we added a js alert when people attempt to bulk delete images. It's been helpful at reducing instances of uninformed bulk deletion, but I still see a fair few cases of people deleting without the understanding that deleting from the media library will also make images unavailable within posts.

The warning currently lives in two places, one of which is filterable and one of which is not. The non-filterable one is the one that actually gets used on the media page, so while I'm happy to just filter the message on my install, it won't actually work. The message reads as follows:

You are about to permanently delete these items.\n 'Cancel' to stop, 'OK' to delete.

The warning is sufficiently dire about the deletion of the images, but if somebody doesn't understand that image removal will affect post content, it doesn't do them much good. Think about how you can embed an image in a Keynote presentation, delete the image from a folder on your hard drive, and it remains embedded in the Keynote without issue. I think that's the expectation some users have. If we can clarify that, I think we can avoid some data loss. I propose something like this:

You are about to permanently delete these items.\nDeleting the files permanently removes them from posts and pages on your site.\n 'Cancel' to stop, 'OK' to delete.

If that seems reasonable, I'm happy to work up a patch.

Attachments (4)

39712.patch (886 bytes) - added by gma992 6 months ago.
Patch attached
39712.2.patch (2.0 KB) - added by gma992 6 months ago.
Message rephrased for different item types.
39712.3.patch (2.1 KB) - added by joemcgill 3 months ago.
39712.4.patch (2.1 KB) - added by joemcgill 3 months ago.

Download all attachments as: .zip

Change History (32)

#1 @azaozz
6 months ago

  • Milestone changed from Awaiting Review to 4.7.4

Sounds like a "low hanging fruit" that will make the UX there better.

@gma992
6 months ago

Patch attached

#2 follow-up: @gma992
6 months ago

Image of applied patch, just added the extra line as @dllh recommended:
https://cldup.com/o7evferxdv.png

Last edited 6 months ago by swissspidy (previous) (diff)

#3 @swissspidy
6 months ago

  • Keywords has-patch added

#4 in reply to: ↑ 2 @azaozz
6 months ago

  • Keywords needs-patch added; has-patch removed

Replying to gma992:

39712.patch is a good start but that string is used in few different places: deleting a post, bulk deleting, deleting tags, etc.

See:
https://core.trac.wordpress.org/browser/tags/4.7.3/src/wp-admin/js/common.js#L90,
https://core.trac.wordpress.org/browser/tags/4.7.3/src/wp-admin/js/tags.js#L8,
https://core.trac.wordpress.org/browser/tags/4.7.3/src/wp-admin/includes/meta-boxes.php#L337,
https://core.trac.wordpress.org/browser/tags/4.7.3/src/wp-admin/includes/class-wp-media-list-table.php#L669.

We have two options:

  • Update the way we show these warnings so each has its own string, then make the strings specific.
  • Change the current string but keep it "good enough" for different "items" like post(s), attachment(s), tag(s), etc. This could be as simple as:

    You are about to permanently delete these items.\nThis will remove them from your site.\n 'Cancel' to stop, 'OK' to delete.

There is also a very similar message (singular and plural) in the media library that also needs updating: https://core.trac.wordpress.org/browser/tags/4.7.3/src/wp-includes/media.php#L3435.

Last edited 6 months ago by azaozz (previous) (diff)

@gma992
6 months ago

Message rephrased for different item types.

#5 @gma992
6 months ago

Thanks for the heads up @azaozz . I went ahead with the (2) recommendation for simplicity sake, keeping it "good enough" for different items, as we only want to strengthen the message that the deletion is permanent.

Last edited 5 months ago by gma992 (previous) (diff)

#6 @azaozz
6 months ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In 40283:

Improve wording of the AYS warning when permanently deleting uploads, tags, posts.

Props dllh, gma992.
Fixes #39712 for trunk.

#7 @azaozz
6 months ago

  • Keywords fixed-major added; needs-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopen for 4.7.4.

This ticket was mentioned in Slack in #core by swissspidy. View the logs.


5 months ago

#9 @swissspidy
5 months ago

  • Owner changed from azaozz to swissspidy
  • Status changed from reopened to reviewing

#10 @swissspidy
5 months ago

@ocean90 raised some concerns during bug scrub:

The string doesn't make much sense to me. Why can't this be combined into one? And I don't think that it's important enough for 4.7.4

#11 follow-up: @ocean90
5 months ago

  • Keywords needs-patch added; fixed-major removed
  • Milestone changed from 4.7.4 to 4.8

And by "doesn't make much sense to me" I mean that the new string is kind of repetitive. I think we can combine this into one sentence: "You are about to permanently delete these items from your site.".

A similar string is used when deleting a user: "Once you hit “Confirm Deletion”, the user will be permanently removed." and "Delete brings you to the Delete Users screen for confirmation, where you can permanently remove a user from your site and delete their content.".

Since I'm not a native English speaker I checked again if there's actually a difference between "delete" and "remove" and found this definition:

Delete and remove are defined quite similarly, but the main difference between them is that delete means erase (i.e. rendered nonexistent or nonrecoverable), while remove connotes take away and set aside (but kept in existence).

So, we should probably not mix them like [40283] does.

#12 in reply to: ↑ 11 ; follow-up: @SergeyBiryukov
5 months ago

Replying to ocean90:

And by "doesn't make much sense to me" I mean that the new string is kind of repetitive. I think we can combine this into one sentence: "You are about to permanently delete these items from your site.".

Being repetitive is not necessarily bad when dealing with destructive actions, it gives user a chance to think twice. However, I agree that perhaps we should not mix "delete" and "remove" in the same message.

How about "You are about to permanently delete these items. They will no longer be available on your site."?

#13 in reply to: ↑ 12 ; follow-up: @azaozz
5 months ago

Replying to SergeyBiryukov:

Being repetitive is not necessarily bad...

I agree. The warning has to be as clear as possible about what is going to happen when the user clicks "OK". I'd rather see it being too long/wordy than concise but maybe a bit unclear for some users.

How about "You are about to permanently delete these items. They will no longer be available on your site."?

That sounds good too imho.

#14 in reply to: ↑ 13 ; follow-up: @ocean90
5 months ago

Replying to azaozz:

what is going to happen when the user clicks "OK".

I think that's the biggest problem here. "OK" is something which a user might click too fast because everything is OK. See also #40128 and #34816. We should really have our own modals to get more options in making such tasks super clear.

#15 in reply to: ↑ 14 ; follow-up: @gma992
5 months ago

Replying to ocean90:

I think that's the biggest problem here. "OK" is something which a user might click too fast because everything is OK. See also #40128 and #34816. We should really have our own modals to get more options in making such tasks super clear.

How about using "Delete" instead of "OK", that will make think twice before pressing "Ok" without reading the warning message:

'Cancel' to stop, 'Delete' to delete.

I'm not quite sure where is this modal coming from, I guess is the default modal and cannot be changed directly? Would we need a custom modal here? I can work on something but I'll appreciate some pointers on where to look or what needs to be changed.

#16 in reply to: ↑ 15 @SergeyBiryukov
4 months ago

Replying to gma992:

I'm not quite sure where is this modal coming from, I guess is the default modal and cannot be changed directly? Would we need a custom modal here?

That's right, see #40128 and #34816.

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


4 months ago

#18 @mikeschroder
4 months ago

We'll need another commit, whether to amend or revert, before 4.8.

#19 @swissspidy
4 months ago

  • Owner swissspidy deleted

I agree that the current wording isn't ideal.

"You are about to permanently delete these items. They will no longer be available on your site." sounds good to me for 4.8.

I'd really love to see custom modals sometime in the future though.

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


3 months ago

@joemcgill
3 months ago

#21 @joemcgill
3 months ago

  • Keywords has-patch 2nd-opinion added; needs-patch removed

39712.3.patch rewords this to read:

You are about to permanently delete this item from your site.
This cannot be undone.
'Cancel' to stop, 'OK' to delete.

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


3 months ago

#23 follow-up: @ocean90
3 months ago

@joemcgill What's the reason for changing the string into singular in wp-includes/script-loader.php? You'll get the message if you delete one or more items in the list view of the media library so plural seems fine here.

@joemcgill
3 months ago

#24 in reply to: ↑ 23 @joemcgill
3 months ago

  • Owner set to joemcgill
  • Status changed from reviewing to accepted

Replying to ocean90:

@joemcgill What's the reason for changing the string into singular in wp-includes/script-loader.php?

Officially...sloppy copy/paste is the reason 🙃. Updated in 39712.4.patch.

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


3 months ago

#26 @ocean90
3 months ago

  • Keywords 2nd-opinion removed

I'd probably change it to "This action cannot be undone", but otherwise 39712.4.patch looks good to me.

#27 @joemcgill
3 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 40650:

Improve wording of the AYS warning when permanently deleting uploads, tags, posts.

This is a follow up on [40283], which cleans up the wording.

Props azaozz, swissspidy, ocean90.
Fixes #39712.

#28 @swissspidy
2 months ago

Followup ticket: #32622

Note: See TracTickets for help on using tickets.