WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#33431 closed defect (bug) (fixed)

"(Currently set to: %s)" string needs context

Reported by: SergeyBiryukov Owned by: ocean90
Milestone: 4.4 Priority: normal
Severity: normal Version: 4.3
Component: Customize Keywords: good-first-bug has-patch
Focuses: Cc:
PR Number:

Description

On the old Menus screen, the string is used for menu locations, and %s is a menu name.

In the Customizer, the same string is used for menus, and %s is a location slug.

On a related note, "(Current: %s)" is missing a translator comment for the placeholder (%s is a menu name).

Attachments (2)

class-wp-customize-control.php.patch (940 bytes) - added by shahpranaf 4 years ago.
Updated Translation Context
33431.diff (1.9 KB) - added by ryankienstra 4 years ago.

Download all attachments as: .zip

Change History (14)

#1 @wonderboymusic
4 years ago

  • Keywords needs-patch added

#2 @DrewAPicture
4 years ago

  • Keywords good-first-bug added

@shahpranaf
4 years ago

Updated Translation Context

#3 @shahpranaf
4 years ago

  • Keywords has-patch added; needs-patch removed

Hi SergeyBiryukov,

If I understood the bug correctly, the context is showing "Current Menu Location" but %s prints Menu name in Customizer.
I've updated the Context. Let me know if "Current menu name" is fine or "Current menu" is correct.

Thanks
Pranav Shah

#4 @pavelevap
4 years ago

@SergeyBiryukov: We are missing context for (Currently set to: %s), but on the other hand we have very similar string (Current: %s) for menu name. What about merging them? I do not see a difference, there is only "set to" part added?

#5 @wonderboymusic
4 years ago

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

#6 @shahpranaf
4 years ago

@SergeyBiryukov Any update on this?

Thanks

#7 follow-up: @ocean90
4 years ago

I think in the Customizer we should show the name too, not the slug.

#8 @wonderboymusic
4 years ago

  • Owner changed from SergeyBiryukov to ocean90

@ryankienstra
4 years ago

#9 @ryankienstra
4 years ago

@ocean90,
Attached is a patch, and here is a GitHub PR with the same edits.

This adds translator information for the menu location.
In data passed to a JavaScript file, it adds translator context after (Currently set to: %s).
And in the Customizer nav menu control, it adds a translator comment to indicate the menu name.

Last edited 4 years ago by ryankienstra (previous) (diff)

#10 @wonderboymusic
4 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 35676:

Customizer i18n: provide translator context for current menu name and location.

Props ryankienstra.
Fixes #33431.

#11 @SergeyBiryukov
4 years ago

In 35722:

Customizer: Use correct context and translator comments for menu location strings.

See #33431.

#12 in reply to: ↑ 7 @SergeyBiryukov
4 years ago

Replying to ocean90:

I think in the Customizer we should show the name too, not the slug.

Yep, moved to a separate ticket: #34755.

Note: See TracTickets for help on using tickets.