WordPress.org

Make WordPress Core

Opened 4 months ago

Last modified 2 weeks 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.7 Priority: normal
Severity: normal Version:
Component: I18N Keywords: has-patch
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 3 months ago.
50773.2.patch (36.2 KB) - added by justinahinon 2 months ago.
50773.3.patch (961 bytes) - added by justinahinon 2 months ago.
Fix strings cases inconsistencies in JS files
50773.4.diff (37.1 KB) - added by dkotter 6 weeks ago.
Combine patches 50773.2 and 50773.3 together
50773.2.diff (95.2 KB) - added by justinahinon 3 weeks ago.

Download all attachments as: .zip

Change History (25)

#1 in reply to: ↑ description @ramiy
4 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
3 months ago

#2 @dkotter
3 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
3 months ago

  • Keywords has-patch needs-testing added

#4 follow-up: @ramiy
3 months ago

Does the patch include translation strings from #50747 ?

#5 in reply to: ↑ 4 @dkotter
2 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.


2 months ago

#7 @helen
2 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
2 months ago

I'll go through both and update the patch.

#9 @justinahinon
2 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
2 months ago

Fix strings cases inconsistencies in JS files

#10 @garrett-eclipse
6 weeks 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
6 weeks ago

Combine patches 50773.2 and 50773.3 together

#11 @dkotter
6 weeks 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
5 weeks ago

  • Keywords needs-refresh removed

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


3 weeks ago

@justinahinon
3 weeks ago

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


3 weeks ago

#16 @SergeyBiryukov
3 weeks ago

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

#17 @audrasjb
3 weeks ago

  • Keywords needs-testing removed

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

#18 @hellofromTonya
3 weeks ago

  • Keywords commit added

#19 @SergeyBiryukov
2 weeks 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
2 weeks ago

#50747 was marked as a duplicate.

Note: See TracTickets for help on using tickets.