WordPress.org

Make WordPress Core

Opened 6 months ago

Last modified 4 months ago

#44674 assigned enhancement

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

Reported by: garrett-eclipse Owned by: idea15
Milestone: Future Release Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: has-patch has-screenshots i18n-change ux-feedback 2nd-opinion
Focuses: administration, privacy 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 (3)

44674.diff (412 bytes) - added by shariqkhan2012 6 months ago.
Change the label from 'Remove' to 'Delete'
Before_Patch.png (30.9 KB) - added by shariqkhan2012 6 months ago.
Before Patch
After_Patch.png (30.7 KB) - added by shariqkhan2012 6 months ago.
After patch

Download all attachments as: .zip

Change History (23)

#1 @desrosj
6 months ago

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

#2 @desrosj
6 months ago

  • Milestone changed from Awaiting Review to Future Release

#3 @garrett-eclipse
6 months ago

  • Keywords good-first-bug added

@shariqkhan2012
6 months ago

Change the label from 'Remove' to 'Delete'

@shariqkhan2012
6 months ago

Before Patch

@shariqkhan2012
6 months ago

After patch

#4 @shariqkhan2012
6 months ago

  • Keywords has-patch added

#5 @shariqkhan2012
6 months 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.


6 months ago

#7 @garrett-eclipse
6 months 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
6 months 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
6 months 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.


6 months ago

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


5 months ago

#12 @desrosj
5 months 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
5 months 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
5 months 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
5 months 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
5 months 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
5 months ago

  • Keywords 2nd-opinion added

#18 @idea15
5 months ago

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

#19 @pento
4 months ago

  • Milestone changed from 4.9.9 to Future Release

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


4 months ago

Note: See TracTickets for help on using tickets.