Make WordPress Core

Opened 7 years ago

Closed 5 years ago

#44674 closed enhancement (fixed)

Privacy Bulk Action 'Remove' should be 'Delete' or 'Move to Trash' to follow convention

Reported by: garrett-eclipse's profile garrett-eclipse Owned by: xkon's profile xkon
Milestone: 5.4 Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: has-patch has-screenshots commit
Focuses: administration Cc:

Description

Hello,

The Privacy tools utilize the verbiage 'Remove' for the delete action of removing requests which is unique aside from MS Users in the case where they remove from a site but aren't deleted. Reviewing other bulk actions on WP_List tables I found the convention is either 'Delete' if the content is removed by the action or 'Move to Trash' if it's archived.

Looking at code this would only be a verbiage change as the underlying action is called 'delete';
https://github.com/WordPress/WordPress/blob/b4320bcd8f9a4566c9c05ebdeaea0908079186f9/wp-admin/includes/user.php#L1152

Other instances of similar bulk actions throughout WP;
Links list table ('Delete') - https://github.com/WordPress/WordPress/blob/aaf99e691391cfceb004d848450dbbf3344b1bee/wp-admin/includes/class-wp-links-list-table.php#L86
MS Sites list table ('Delete') - https://github.com/WordPress/WordPress/blob/99e3bb9077c5af8dcb421332be20e2f7558b9145/wp-admin/includes/class-wp-ms-sites-list-table.php#L204
Users list table ('Delete' unless in MS it's 'Remove' as they're removing from site and not deleting from system) - https://github.com/WordPress/WordPress/blob/1b5d6c6971c158e38b2f7c4ac6acfa19276aa5df/wp-admin/includes/class-wp-users-list-table.php#L240
Terms list table ('Delete') - https://github.com/WordPress/WordPress/blob/b76a714bba8c7373e443afab47d02d3d9df2aaa3/wp-admin/includes/class-wp-terms-list-table.php#L155

For the time being 'Delete' makes the most sense as it aligns with the other list tables when there's no trash archive. If #44222 is implemented to add an archive/trash state to the workflow then the convention would be 'Move to Trash' and 'Delete Permanently' for the two actions depending if you're in the main view or archive/trash view.

Thanks

Attachments (8)

44674.diff (412 bytes) - added by shariqkhan2012 7 years ago.
Change the label from 'Remove' to 'Delete'
Before_Patch.png (30.9 KB) - added by shariqkhan2012 7 years ago.
Before Patch
After_Patch.png (30.7 KB) - added by shariqkhan2012 7 years ago.
After patch
44674.2.diff (607 bytes) - added by xkon 5 years ago.
44674.2_preview.jpg (19.0 KB) - added by xkon 5 years ago.
44674.3.diff (621 bytes) - added by xkon 5 years ago.
44674.3_preview.jpg (24.0 KB) - added by xkon 5 years ago.
44674.4.diff (551 bytes) - added by garrett-eclipse 5 years ago.
Update 'Re-send Confirmation Emails' to 'Resend Confirmation Requests'

Download all attachments as: .zip

Change History (35)

#1 @desrosj
7 years ago

  • Focuses administration added
  • Keywords ux-feedback added; i18n-change removed

#2 @desrosj
7 years ago

  • Milestone changed from Awaiting Review to Future Release

#3 @garrett-eclipse
7 years ago

  • Keywords good-first-bug added

@shariqkhan2012
7 years ago

Change the label from 'Remove' to 'Delete'

@shariqkhan2012
7 years ago

Before Patch

@shariqkhan2012
7 years ago

After patch

#4 @shariqkhan2012
7 years ago

  • Keywords has-patch added

#5 @shariqkhan2012
7 years ago

Since this is my first patch and it is related to wording/verbiage/text, I am also curious to know if one needs to do any other changes to handle the translation stuff too

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


7 years ago

#7 @garrett-eclipse
7 years ago

  • Keywords needs-testing has-screenshots i18n-change added; good-first-bug removed

Thanks @shariqkhan2012, welcome to contributing. Your patch looks great. And looks like you got the answer to translations in Slack but will quickly re-iterate that any strings within translation functions will automatically be parsed. Cheers

#8 @shariqkhan2012
7 years ago

Thank you and the other volunteers who answered my queries. T took up this one to get a hang of things. I hope to make more contributions.
BTW, if I add a patch to an issue, do I have to take additional actions other than adding the keyword 'has-patch'? Just curious to know how an issue gets included in the release cycles

#9 @garrett-eclipse
7 years ago

  • Milestone changed from Future Release to 4.9.9

That's great I hope to see you around more.

The keyword is a great start and I've updated this from 'Future Release' to 4.9.9 so we can look into including it in the next release. Also if you have a ticket you feel ready and want in a cycle you can hop onto Slack in the #core channel during office-hours and raise it for discussion. You can also join us in #core-privacy to discuss this specific ticket as it's part of the Privacy component.

This thread will update as it makes it way through the cycle and if there's any amendments you'll be notified.

Cheers

This ticket was mentioned in Slack in #core-privacy by garrett-eclipse. View the logs.


7 years ago

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


6 years ago

#12 @desrosj
6 years ago

  • Keywords commit added; ux-feedback needs-testing removed

Thanks for the patch, @shariqkhan2012! This looks like a sensible change to me. The patch looks good as well.

#13 @websupporter
6 years ago

With a client we are currently using the eraser extensively. Actually, we generate erase requests, when user do not login for a certain amount of time. The XP of my client was that the "Remove" in the select would "Remove" the user and he was confused why he could again "Remove" the user in the "Completed"-tab.

I told him "Remove" removes the request and only the button is to actually erase the user data. I saw this ticket and thought another naming would help to limit the confusion, but "Delete" provokes just more confusion (or the same at least).

"Move To Trash" would make me wonder, where is the trash, so this doesn't look like an option to me. But maybe "Delete the Request" or something, so it becomes more clear, what this option is about?

Thank you for your efforts :)

#14 @desrosj
6 years ago

  • Keywords ux-feedback added; commit removed

Thanks for your feedback, @websupporter. That is useful information. I removed the commit keyword so that we can get more feedback.

@joshuawold @karmatosed are you able to take a look and make a recommendation here?

#15 @JoshuaWold
6 years ago

I'm torn on this. Really appreciate the great thoughts here!

  1. "Delete request" seems to make the most sense, if all we're ever doing with this command is truly deleting requests.
  2. I didn't see an example where a removed request shows up anywhere else, in fact it seems that they disappear completely from the system, so delete seems appropriate here.
  3. Whatever wording we use for the dropdown should also be used for the confirmation message (it currently says "deleted X request")

#16 @shariqkhan2012
6 years ago

My 2 cents:

  1. Just 'Delete' is consistent with the usage on other screens. For eg. 'Users', 'Posts' screens just use the word 'Delete', and not something like 'Delete User' or 'Delete Post'
  1. Having said that, I can see @websupporter 's point of view. It could be a bit confusing for clients as he explained to understand what is the entity that is going to be deleted. Especially more so, because the 'privacy' feature is quite new as compared to the other screens which have there from almost since the beginning.
  1. One option could be to make this information more prominent in the 'Help' section associated with that screen.

#17 @idea15
6 years ago

  • Keywords 2nd-opinion added

#18 @idea15
6 years ago

  • Owner set to idea15
  • Status changed from new to assigned

#19 @pento
6 years ago

  • Milestone changed from 4.9.9 to Future Release

This ticket was mentioned in Slack in #core-privacy by webdevlaw. View the logs.


6 years ago

#21 @garrett-eclipse
6 years ago

  • Focuses privacy removed
  • Keywords needs-refresh added; i18n-change removed

#22 @xkon
5 years ago

  • Owner changed from idea15 to xkon

@xkon
5 years ago

@xkon
5 years ago

#23 @xkon
5 years ago

  • Keywords ux-feedback needs-refresh removed
  • Milestone changed from Future Release to 5.4

Right, this makes total sense as it's confusing at the moment.

44674.2.diff changes the dropdown texts to Delete Requests and Resend Emails as seen on 44674.2_preview.jpg .

It's in plural as single-row requests will be added as well on ticket #44266 for both emails and deletions 🙂.

I'm marking this for 5.4 as it's a simple change, and I'll see if I can manage to get a patch going for the single row actions asap.

@xkon
5 years ago

@xkon
5 years ago

#24 @xkon
5 years ago

By introducing the inline row actions at #44266 it seems logical to be fully clear as well on the "emails" that are sent as the action will only re-send the Confirmation emails to re-start the process.

This will avoid extra confusion if a request is at the "send export file" stage since that's via email as well.

44674.3.diff changes the bulk action names to Delete Requests and Re-send Confirmation Emails

@garrett-eclipse
5 years ago

Update 'Re-send Confirmation Emails' to 'Resend Confirmation Requests'

#25 @garrett-eclipse
5 years ago

  • Keywords 2nd-opinion removed

Thanks for the patch @xkon I've made a minor refresh in 44674.4.diff which changed Re-send Confirmation Emails to Resend Confirmation Requests.
*English is weird... you use a dash when if an email was re-sent but don't use it when you resend an email. This is because re-send isn't a word. And it's re-sent to avoid confusion with resent which is when you dislike/hate something.
I went with Confirmation Requests instead of Confirmation Emails as there's the possibility of SMS delivery (some plugins replace email sends with SMS) and it matches the other messages such as the success on this action which states 'Re-sent %n request(s)'.

Take a look and if you're happy with that let's mark for commit.

#26 @xkon
5 years ago

  • Keywords commit added

Sure that's totally fine with me :-) and thanks for the explanation!

We should also change the wording on #44266 then for the row actions!

Marking for commit.

Last edited 5 years ago by xkon (previous) (diff)

#27 @SergeyBiryukov
5 years ago

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

In 47148:

Privacy: Clarify bulk action labels for personal data export and removal requests.

Props garrett-eclipse, xkon, shariqkhan2012, websupporter, desrosj, JoshuaWold.
Fixes #44674.

Note: See TracTickets for help on using tickets.