Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce 'template_name' label to custom post types and custom taxonomies #6344

Closed

Conversation

Aljullu
Copy link

@Aljullu Aljullu commented Apr 2, 2024

Trac ticket: https://core.trac.wordpress.org/ticket/60881

This PR is a follow-up of WordPress/gutenberg#60367 to:

  1. Document the template_name label when registering post types and taxonomies.
  2. Add a default value.

Testing steps

  1. Add this code snippet to your site. You can use the Code Snippets plugin:
add_action( 'init', function () {
	register_post_type(
		'recipe',
		array(
			'labels'       => array(
				'name'          => 'Recipes',
				'singular_name' => 'Recipe',
			),
			'public'       => true,
			'show_in_rest' => true,
		)
	);
	register_taxonomy(
		'recipe_cat',
		'recipe',
		array(
			'label'        => 'Categories',
			'labels'       => array(
				'name'          => 'Recipe categories',
				'singular_name' => 'Category',
			),
			'show_in_rest' => true,
		)
	);
} );
  1. Go to Appearance > Editor > Templates > Add new and verify there are no regressions in the Add template screen: two items related to the code snippet should appear there: Single item: Recipe and Category (recipe_cat).

Add template screen with default labels

  1. Install Gutenberg and checkout this branch: Update 'Add template' screen to prefer template_name label instead of singular_name gutenberg#60367.
  2. Repeat step 2 and verify there are no regressions.
  3. Now, modify the code snippet you created before, adding template_name to the post type and taxonomy:
add_action( 'init', function () {
	register_post_type(
		'recipe',
		array(
			'labels'       => array(
				'name'          => 'Recipes',
				'singular_name' => 'Recipe',
+				'template_name' => 'Single recipe',
			),
			'public'       => true,
			'show_in_rest' => true,
		)
	);
	register_taxonomy(
		'recipe_cat',
		'recipe',
		array(
			'label'        => 'Categories',
			'labels'       => array(
				'name'          => 'Recipe categories',
				'singular_name' => 'Category',
+				'template_name' => 'Recipes by category',
			),
			'show_in_rest' => true,
		)
	);
} );
  1. Repeat step 2 and verify the user-friendly template labels are shown:

Add template screen with user-friendly labels


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

Copy link

github-actions bot commented Apr 2, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props aljullu, peterwilsoncc, audrasjb, ntsekouras.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link

github-actions bot commented Apr 2, 2024

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@ntsekouras
Copy link

Thanks for the PR! Can you create a trac ticket for this and link this PR?

@Aljullu
Copy link
Author

Aljullu commented May 29, 2024

Can you create a trac ticket for this and link this PR?

Thanks for taking a look, @ntsekouras! The track ticket was already created: https://core.trac.wordpress.org/ticket/60881. It's linked from the PR description. 🙂

Copy link
Contributor

@audrasjb audrasjb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright this looks good to me 👍

@Aljullu Aljullu force-pushed the 60881-introduce-template_name-label branch from 16f1d8d to 999985e Compare May 29, 2024 14:17
@Aljullu
Copy link
Author

Aljullu commented May 29, 2024

Thanks for the review, @audrasjb! I rebased this PR with the latest trunk but I don't have permissions to merge. I also see some checks were cancelled or skipped, is that expected?

Copy link
Contributor

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will cause a regression for custom taxonomies.

Currently the singular name is used by default, so it's clear the taxonomy the template is for the human label, ie the one shown in the admin.

Screen Shot 2024-05-30 at 9 38 57 am

After these chanages (and some might be upstream) the label will become Tags (taxonomy_slug) which is much more difficult to interpret as a user.

I think the default should be Singular Name Archives so the human name remains in the site editor. The logic for defining the default label would probably need to be in get_taxonomy_labels instead.

src/wp-includes/class-wp-taxonomy.php Outdated Show resolved Hide resolved
src/wp-includes/taxonomy.php Show resolved Hide resolved
@Aljullu Aljullu force-pushed the 60881-introduce-template_name-label branch from ffb78b9 to 4f06e99 Compare May 30, 2024 09:56
@Aljullu
Copy link
Author

Aljullu commented May 30, 2024

Thanks for the review, @peterwilsoncc! I updated the PR based on your feedback. If the custom post type and the custom taxonomies don't include a template_name, Single item: [singular name] and [singular name] Archives will be used instead:

imatge

@Aljullu Aljullu requested a review from peterwilsoncc May 30, 2024 10:20
Copy link
Contributor

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a few more notes inline.

While adding the labels, do you think it's worth adding one for post type archives too?

At the moment the archive is always shown as Archive: [CPT] but if WP is to allow for Single item: [CPT] to be customized it seems inconsistent not to allow the archive to be renamed too.

Let me know if this was discussed on the upstream ticket.

Screen Shot 2024-05-31 at 11 28 01 am

This is the mini-plugin I was using for testing this change:

add_action( 'init', function() {
	register_post_type( 'pwcc_test_type', [
		'labels' => [
			'name'          => 'Test CPTs',
			'singular_name' => 'Test CPT',
			'template_name' => 'Template Name Test CPT',
		],
		'public' => true,
		'has_archive' => true,
		'show_in_rest' => true,
	] );

	register_taxonomy( 'pwcc_test_taxonomy', 'pwcc_test_type', [
		'label' => 'Test Taxonomy',
		'labels' => [
			'name'          => 'Test Taxonomies',
			'singular_name' => 'Test Taxonomy',
			'template_name' => 'Template Name Test Taxonomy',
		],
		'show_in_rest' => true,
	] );
} );

src/wp-includes/class-wp-taxonomy.php Outdated Show resolved Hide resolved
src/wp-includes/taxonomy.php Outdated Show resolved Hide resolved
@Aljullu
Copy link
Author

Aljullu commented May 31, 2024

Thanks again for the review, @peterwilsoncc, feedback should be addressed.

At the moment the archive is always shown as Archive: [CPT] but if WP is to allow for Single item: [CPT] to be customized it seems inconsistent not to allow the archive to be renamed too.

You mean adding a archive_template_name label or something along these lines? AFAIK it hasn't been discussed, as I didn't think about this case. I'm happy to add it, but I would lean towards doing that in a separate PR to keep the scope of this one smaller, if that sounds good.

@Aljullu Aljullu requested a review from peterwilsoncc May 31, 2024 12:17
Copy link
Contributor

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This look good to me, thank you!

Testing notes:

✅ Setting the template_name displays the string in the add new template screen
✅ Setting labels without the template name displays the singular name
✅ Setting neither $label or $labels displays the object slug to differentiate
✅ Setting singular $label without $labels displays the $label in the default string

@Aljullu
Copy link
Author

Aljullu commented Jun 4, 2024

Once again, thanks for the review, @peterwilsoncc! I don't have merge/commit access to the WordPress repo, so please let me know if there is anything else I can do from my end. 🙏

@peterwilsoncc peterwilsoncc added the props-bot Adding this label triggers the Props Bot workflow for a PR. label Jun 10, 2024
@github-actions github-actions bot removed the props-bot Adding this label triggers the Props Bot workflow for a PR. label Jun 10, 2024
@peterwilsoncc
Copy link
Contributor

Committed r58377 / 7d00db4

@Aljullu Aljullu deleted the 60881-introduce-template_name-label branch June 11, 2024 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants