WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#41150 closed enhancement (fixed)

post_categories_meta_box() function should use a taxonomy label for "Most used" tab text

Reported by: jlambe Owned by: SergeyBiryukov
Milestone: 4.9 Priority: normal
Severity: normal Version:
Component: Taxonomy Keywords: has-patch
Focuses: ui Cc:
PR Number:

Description

This issue is regarding the auto-generated metabox for a custom taxonomy.

When defining a custom hierarchical taxonomy in another language (in this case French), we do not have control of the text label displayed into the "Most used" tab.

When looking at source code, the first tab is calling this in order to display a custom label text for the first tab:

    <?php echo $taxonomy->labels->all_items; ?>

It grabs the taxonomy labels property and use its all_items property where the second tab is only echoing this:

    <?php _e( 'Most Used' ); ?>

It could be a real enhancement to introduce a new label property for the register_taxonomy function. Like most_used for example so users can edit that second tab label text. With this idea, the above line code could be for this second tab:

    <?php echo $taxonomy->labels->most_used; ?>

Our issue here is that the French translation is showing a female word translation (not sure how to say that in English but here is the translation "Les plus utilisées"). A user experience issue is appearing here if we define a male taxonomy name because the translation should be "Les plus utilisés".

On the other side, I do think it's not a good to globally change the Most used text label and the idea of having a new label most_used property per taxonomy could improve the user experience.

What do you think?

Attachments (4)

41150.diff (1.4 KB) - added by mdifelice 2 years ago.
41150.1.diff (2.2 KB) - added by mdifelice 2 years ago.
Added documentation.
41150.2.diff (777 bytes) - added by purnendu 2 years ago.
context removed
41150.3.diff (2.0 KB) - added by johnbillion 2 years ago.

Download all attachments as: .zip

Change History (25)

#1 @johnbillion
2 years ago

  • Focuses accessibility removed
  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to 4.9
  • Version trunk deleted

Yep, let's do this.

#2 @ajay188843
2 years ago

I think this issue about the language translation not of the coading... am i right ?

Last edited 2 years ago by ajay188843 (previous) (diff)

@mdifelice
2 years ago

#3 @mdifelice
2 years ago

  • Keywords has-patch added; needs-patch removed

The patch I sent address the issue.

It is worth to note that some outputs on the post_categories_meta_box() function do not respect the late escaping policy neither escape at all.

#4 follow-up: @DrewAPicture
2 years ago

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

Assigning ownership to mark the good-first-bug as "claimed".

#5 in reply to: ↑ 4 ; follow-up: @mdifelice
2 years ago

Replying to DrewAPicture:

Assigning ownership to mark the good-first-bug as "claimed".

Hi @DrewAPicture. Sorry to bother you but I am new with this. So I have a couple of questions:

  1. Should I assign myself the ticket in cases like this or should I wait someone to do it? Like a Core Committer?
  2. What should be the next steps in order to get this committed? Making tests or something else?

Thanks!

#6 in reply to: ↑ 5 @DrewAPicture
2 years ago

Replying to mdifelice:

  1. Should I assign myself the ticket in cases like this or should I wait someone to do it? Like a Core Committer?

Nope! You'd just wait for a bug gardener (not all bug gardeners are committers) to assign you. Note that this assignment workflow only really applies to tickets with the good-first-bug keyword – the ownership field is what signifies a ticket as "claimed" on the Good First Bugs report, that's all.

  1. What should be the next steps in order to get this committed? Making tests or something else?

The next step is to wait for feedback from @johnbillion (who milestoned this ticket), other contributors, or any of the Taxonomy component maintainers. If, after a few days nobody has responded, you could (politely!) ping one of the maintainers on Slack to try to inspire some movement.

Hope that helps clarify the process for you :-)

Last edited 2 years ago by DrewAPicture (previous) (diff)

#7 @boonebgorges
2 years ago

  • Keywords needs-docs added

@mdifelice Thanks for the patch!

It is worth to note that some outputs on the post_categories_meta_box() function do not respect the late escaping policy neither escape at all.

True, but we should be strict about this with all newly added strings. (Fixing the old ones should be a separate project, and will be complicated by backward compatibility concerns.)

The approach in 41150.diff looks good. Could you please add some documentation to get_taxonomy_labes() that describes the new string? See the function docblock for examples (both the @since and @return annotations), and see https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/#1-1-parameters-that-are-arrays and https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/#since-section-changelogs for more on our documentation standards. Thank you!

@mdifelice
2 years ago

Added documentation.

#8 @mdifelice
2 years ago

Hi @boonebgorges. Thanks for your feedback. I sent another version. What do you think? I am not sure what to put in the @since parameter.

Regards!

#9 @boonebgorges
2 years ago

  • Keywords needs-docs removed

@mdifelice Thanks for the quick turnaround! I'll fix the version number.

#10 @boonebgorges
2 years ago

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

In 40984:

Introduce most_used taxonomy label, for text on 'Most Used' metabox tab.

Props mdifelice, jlambe.
Fixes #41150.

#11 @mdifelice
2 years ago

Great @boonebgorges!

#12 @johnbillion
2 years ago

  • Keywords needs-patch added; has-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

I think this new label should have either a translators: comment, or context via _x(), or both.

#13 @johnbillion
2 years ago

The reason for this is because the taxonomy name isn't included in the string.

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


2 years ago

#15 @johnbillion
2 years ago

  • Owner changed from mdifelice to johnbillion
  • Status changed from reopened to assigned

#16 @johnbillion
2 years ago

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

In 41674:

Taxonomy: Add context and a translator comment to the new most_used label.

Fixes #41150

#17 @SergeyBiryukov
2 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

The same string is used without a context in wp_nav_menu_item_taxonomy_meta_box(), for both hierarchical and non-hierarchical taxonomies.

We should either add the label for non-hierarchical taxonomies as well, or remove the context, as it just duplicates the string for no reason, causing some confusion for translators.

@purnendu
2 years ago

context removed

#18 @purnendu
2 years ago

  • Keywords has-patch added; needs-patch removed

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


2 years ago

@johnbillion
2 years ago

#20 @johnbillion
2 years ago

  • Keywords good-first-bug removed
  • Owner changed from johnbillion to SergeyBiryukov
  • Status changed from reopened to reviewing

41150.3.diff improves the context for clarity, adds the label for non-hierarchical taxonomies too, and uses the label on the nav menus screen too.

#21 @SergeyBiryukov
2 years ago

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

In 41987:

Taxonomy: After [40984], add the most_used label for non-hierarchical taxonomies too, and use it on the Menus screen.

Props johnbillion.
Fixes #41150.

Note: See TracTickets for help on using tickets.