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: |
|
Owned by: |
|
---|---|---|---|
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
- Comments list table ('Move to Trash' & 'Delete Permanently') - https://github.com/WordPress/WordPress/blob/1b5d6c6971c158e38b2f7c4ac6acfa19276aa5df/wp-admin/includes/class-wp-comments-list-table.php#L328
- Plugins list table ('Delete') - https://github.com/WordPress/WordPress/blob/1b5d6c6971c158e38b2f7c4ac6acfa19276aa5df/wp-admin/includes/class-wp-plugins-list-table.php#L461
- Posts list table ('Move to Trash' & 'Delete Permanently') - https://github.com/WordPress/WordPress/blob/1b5d6c6971c158e38b2f7c4ac6acfa19276aa5df/wp-admin/includes/class-wp-posts-list-table.php#L397
- Media list table ('Delete Permanently', but if 'MEDIA_TRASH' enabled also has 'Trash' which I'm flagging in another ticket - https://github.com/WordPress/WordPress/blob/8e01f9f99b9cc39bed623543f77752c819132699/wp-admin/includes/class-wp-media-list-table.php#L147
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)
Change History (35)
#5
@
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
@
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
@
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
@
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
@
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
@
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
@
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
@
6 years ago
I'm torn on this. Really appreciate the great thoughts here!
- "Delete request" seems to make the most sense, if all we're ever doing with this command is truly deleting requests.
- 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.
- Whatever wording we use for the dropdown should also be used for the confirmation message (it currently says "deleted X request")
#16
@
6 years ago
My 2 cents:
- 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'
- 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.
- One option could be to make this information more prominent in the 'Help' section associated with that screen.
This ticket was mentioned in Slack in #core-privacy by webdevlaw. View the logs.
6 years ago
#23
@
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.
#24
@
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
#25
@
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.
Change the label from 'Remove' to 'Delete'