Opened 7 months ago
Last modified 8 days ago
#50773 reviewing defect (bug)
Merge duplicate strings in bulk actions and row actions for list tables
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.8 | Priority: | normal |
Severity: | normal | Version: | |
Component: | I18N | Keywords: | has-patch |
Focuses: | ui, administration | Cc: |
Description
As noted by @ramiy in comment:25:ticket:40244 and me in comment:5:ticket:50747, as part of the changes in [48352] and [48595] some strings were duplicated with a different case:
_x( 'Not Spam', 'comment' )
_x( 'Not spam', 'comment' )
_x( 'Not Spam', 'site' )
_x( 'Not spam', 'site' )
Additionally, some strings kept title casing and should be switched to sentence casing for consistency with others:
__( 'Enable Auto-updates' )
__( 'Disable Auto-updates' )
__( 'Network Activate' )
__( 'Network Deactivate' )
__( 'Network Enable' )
__( 'Network Disable' )
The problem is that some of these strings are not only used in bulk actions, but also in row actions.
While most of bulk action strings appear to be switched to sentence casing across the admin now, row actions are not. It seems like that would be the next step to bring some consistency here.
Attachments (5)
Change History (26)
#1
in reply to:
↑ description
@
7 months ago
#2
@
6 months ago
I've gone through all of the list-table
classes and updated strings to use sentence case, where they were previously using title case.
This should take care of the "Row Actions" but might impact other areas as well. If the goal is to tackle those strings in a different ticket, I can clean up my patch to focus only on the "Row Actions". Just seemed to make the most sense to fix all of those at once.
This ticket was mentioned in Slack in #core by helen. View the logs.
5 months ago
#7
@
5 months ago
I see some changes on the latest patch in #50747 that aren't in this patch - could we perhaps pick one ticket and merge patches together? And a review for consistency across these UIs would be nice :)
#9
@
5 months ago
I've merged https://core.trac.wordpress.org/attachment/ticket/50773/50773.diff with https://core.trac.wordpress.org/attachment/ticket/50747/50747.2.patch.
However, there are others occurences of this inconsistencies in js files. I have an upcoming patch tomorrow.
#10
@
4 months ago
- Keywords needs-refresh added
@justinahinon it appears the latest 50773.3.patch only contains the JS strings and is missing all the changes found in 50773.2.patch. Do you have time to merge these so we have a single patch to review/test?
#11
@
4 months ago
@garrett-eclipse @justinahinon I noticed the same while testing this ticket earlier today. I've gone ahead and combined those two patches together into a new patch.
#13
@
4 months ago
#50747 is a duplicate of this ticket. Per this scrub, this ticket is the commit
candidate.
Note to committer:
When this ticket is committed, please add props from ticket #50747.
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 months ago
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 months ago
#17
@
4 months ago
- Keywords needs-testing removed
Thanks @justinahinon 🙌 patch looks ready now, marking this for commit as per today's bug scrub
#19
@
3 months ago
- Keywords commit removed
- Milestone changed from 5.6 to 5.7
Somes notes on 50773.2.diff:
- With the capitalization removed, some of the strings would benefit from quotes. Otherwise, in sentences like "...and click Install now when you are prompted in the popup window", it's not quite clear that the name of the button is "Install now" and not "Install". Or in "click the Upload plugin button", it's not clear that the name is "Upload Plugin" and not "Upload".
- Looking at the change in
src/wp-admin/includes/file.php
, the new "Search results" string is inconsistent with "Posts Page", "Date Template", etc. The other strings will likely need a change too. “%s” (Edit)
should probably be left as is.
Would like to take some more time to review this carefully. Moving to the next release for now.
Replying to SergeyBiryukov:
I agree, the next logical step is to use sentence casing in "Row Actions".