WordPress.org

Make WordPress Core

Opened 15 months ago

Last modified 5 months ago

#50773 reviewing defect (bug)

Merge duplicate strings in bulk actions and row actions for list tables

Reported by: SergeyBiryukov Owned by: SergeyBiryukov
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: I18N Keywords: has-patch needs-testing needs-testing-info
Focuses: ui, administration Cc:

Description

Backround: #40244, #50747.

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)

50773.diff (35.4 KB) - added by dkotter 14 months ago.
50773.2.patch (36.2 KB) - added by justinahinon 13 months ago.
50773.3.patch (961 bytes) - added by justinahinon 13 months ago.
Fix strings cases inconsistencies in JS files
50773.4.diff (37.1 KB) - added by dkotter 12 months ago.
Combine patches 50773.2 and 50773.3 together
50773.2.diff (95.2 KB) - added by justinahinon 12 months ago.

Download all attachments as: .zip

Change History (31)

#1 in reply to: ↑ description @ramiy
15 months ago

Replying to SergeyBiryukov:

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.

I agree, the next logical step is to use sentence casing in "Row Actions".

@dkotter
14 months ago

#2 @dkotter
14 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.

#3 @garrett-eclipse
14 months ago

  • Keywords has-patch needs-testing added

#4 follow-up: @ramiy
14 months ago

Does the patch include translation strings from #50747 ?

#5 in reply to: ↑ 4 @dkotter
13 months ago

Replying to ramiy:

Does the patch include translation strings from #50747 ?

Yes, this contains those updates

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


13 months ago

#7 @helen
13 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 :)

#8 @justinahinon
13 months ago

I'll go through both and update the patch.

#9 @justinahinon
13 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.

@justinahinon
13 months ago

Fix strings cases inconsistencies in JS files

#10 @garrett-eclipse
12 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?

@dkotter
12 months ago

Combine patches 50773.2 and 50773.3 together

#11 @dkotter
12 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.

#12 @JeffPaul
12 months ago

  • Keywords needs-refresh removed

#13 @hellofromTonya
12 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.


12 months ago

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


12 months ago

#16 @SergeyBiryukov
12 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#17 @audrasjb
12 months ago

  • Keywords needs-testing removed

Thanks @justinahinon 🙌 patch looks ready now, marking this for commit as per today's bug scrub

#18 @hellofromTonya
12 months ago

  • Keywords commit added

#19 @SergeyBiryukov
11 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.

#20 @SergeyBiryukov
11 months ago

#50747 was marked as a duplicate.

#21 @lukecarbis
8 months ago

  • Milestone changed from 5.7 to 5.8

Since we're in 5.7 beta 3, let's move this to the next release again.

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


5 months ago

#23 @chaion07
5 months ago

We reviewed this ticket during a recent [bug-scrub session]https://wordpress.slack.com/archives/C02RQBWTW/p1622579544172100. Pinging @SergeyBiryukov to see if you have been able to review this more closely.Really hoping for a feedback for this by Beta 1(June 8) so that a patch refresh can be obtained. Otherwise it is likely that this ticket is getting punted to Future Release. Thanks!

Props to @jeffpaul https://wordpress.slack.com/archives/C02RQBWTW/p1622579639172900

#25 @justinahinon
5 months ago

Hi @SergeyBiryukov
I've just sent this PR https://github.com/WordPress/wordpress-develop/pull/1340 with your last suggestions.

#26 @hellofromTonya
5 months ago

  • Keywords needs-testing needs-testing-info added
  • Milestone changed from 5.8 to 5.9

Today is 5.8 Beta 1. Thank you @justinahinon for submitting the PR. Unfortunately, ran out of time to review and test. Punting to 5.9 where work can continue.

Note: See TracTickets for help on using tickets.