Custom Category Block Template Update

[ChrisMKindred]

Updates the block category template to use the new Custom block category.

See SquareOne PR 1011

[defunctl]

There's a major problem I see with all of this we need to address:

Existing projects that would upgrade to a new version of tribe libs that would then contain this update, will be generating broken block config's via wp s1 generate block <name> because they don't have access to either the Tribe\Project\Blocks\Block_Category::CUSTOM_BLOCK_CATEGORY_SLUG constant itself, nor the logic that registers the new block category in WordPress because those are in square-one!

Important!

NOTE: I added this section after I wrote up all the other things in this review up, but I still think the rest is great for learning, so I left it all in not to confuse you, but to present multiple ways to solve the same problem.

Additional solutions

Option 1

1. Remove the category index completely from our blocks, example of a line to delete.
2. Use the acf/register_block_type_args filter to instead check if it's a block with ACF's default category (common), and replace with our default one, still utilizing the additional filter I suggest below.
3. Add a Block_Subscriber in tribe libs to run this filter, and include it in the square one PR's Core.php (details about how to do this below in the original solution).

That class would look like this:

<?php declare(strict_types=1);

namespace Tribe\Libs\ACF;

class Block_Category {

	public const CUSTOM_BLOCK_CATEGORY_SLUG = 'tribe-custom';
	public const DEFAULT_ACF_CATEGORY      = 'common';

	/**
	 * Set a default block category for our custom blocks.
	 *
	 * @filter acf/register_block_type_args
	 *
	 * @param  mixed[]  $args The block argument.
	 *
	 * @return array
	 */
	public function set_default_category( array $args ): array {
		if ( $args['category'] !== self::DEFAULT_ACF_CATEGORY ) {
			return $args;
		}

		$args['category'] = apply_filters( 'tribe/project/block/config/default_category', self::CUSTOM_BLOCK_CATEGORY_SLUG );

		return $args;
	}

	/**
	 * Adds the Custom Category to the front of the block list categories.
	 *
	 * @filter block_categories
	 *
	 * @param array $categories
	 *
	 * @return array
	 */
	public function custom_block_category( array $categories ): array {
		return array_merge( [
			[
				'slug'  => self::CUSTOM_BLOCK_CATEGORY_SLUG,
				'title' => esc_html__( 'Custom', 'tribe' ),
				'icon'  => '<svg enable-background="new 0 0 146.3 106.3" version="1.1" viewBox="0 0 146.3 106.3" xml:space="preserve" xmlns="http://www.w3.org/2000/svg"><style type="text/css">.st0{fill:#16D690;}.st1{fill:#21A6CB;}.st2{fill:#008F8F;}</style><polygon class="st0" points="145.2 106.3 72.6 42.3 26.5 1.2 0 106.3"/><polygon class="st1" points="145.2 106.3 0 106.3 72.6 42.3 118.6 1.2"/><polygon class="st2" points="72.6 42.3 145.2 106.3 0 106.3"/></svg>',
			],
		], $categories );
	}

}

And the required entry in a subscriber:

		add_filter( 'acf/register_block_type_args', function ( $args ): array {
			return $this->container->get( Block_Category::class )->set_default_category( (array) $args );
		}, 10, 1 );

Cons to this implementation:

1. It's kind of behind the scenes magic, and we'd need to actually document how block categories are set.
2. No blocks could actually be in the common category if they needed to be because ACF sets that as a default and doesn't provide a filter earlier in execution so that we could simply change that.

Option 2

If we really want the definition inside each block's config, I think my class constant suggestion was sort sighted because it is still required to change every single block if the only want to change it is to override the class in the container.

Instead, utilizing WordPress's hook system might be best, e.g. in a Block_Config:

		$this->set_block( new Block( self::NAME, [
			'title'    => esc_html__( 'Logos', 'tribe' ),
			'keywords' => [ esc_html__( 'logos', 'tribe' ) ],
			'category' => apply_filters( 'tribe/project/block/config/default_category', '', $this ),
			'supports' => [
				'align'  => false,
				'anchor' => true,
			],
// more code...

And then in a subscriber in Tribe Libs:

		add_filter( 'tribe/project/block/config/default_category', function ( string $category ): string {
			// Note: this check should be in the class itself, but just putting it here for this review
			if ( $category ) {
				return $category;
			}

			return $this->container->get( Block_Category::class )->set_category_name( $category );
		}, 10, 1 );

That way, it can be overridden on a per project or even a per block basis. If I were to document that filter it would look like this:

		/**
		 * Set's the default block category if not already set.
		 *
		 * @param string                       $category The current block's category.
		 * @param \Tribe\Libs\ACF\Block_Config $block    The current block's instance.
		 */
		$block_category = apply_filters( 'tribe/project/block/config/default_category', '', $this );

---

Original proposed solution / original review write up

The following is a decent option, because:

1. We wouldn't have to bump this to a major 4.0.0 (incompatible change) version, and worry about older projects with bad composer versioning upgrading to this without noticing (e.g. running a composer update and generating broken block configs and causing extra confusion and support.
2. Older projects (non pimple container versions of course) would actually be able to make use of this change by only bringing in the new subscriber to their project's Core.php. So still likely some confusion/support still required here (hopefully once we have docs/faq it will help take care of this kind of situation).
3. Putting this kind of logic into tribe-libs means we can update it for all projects if there is a problem or a change to WP core, and we can certainly see this as a dependency for generators and blocks in general of which the core logic for those are in tribe libs.

Original Recommended Solution

1. Move the Block Category Class into tribe libs (keeping in mind tribe libs is still PHP 7.2, so you may need to adjust the code slightly).
2. Add a new Block_Subscriber to the ACF library, and register it in Core.php in [your square one PR](Field Name and Icon Consistency  square-one#1011 (you can temporarily test this in square-one by adjusting the tribe libs version in the composer.json to this branch, e.g. dev-feature/block-config-templete-category or by simply cloning tribe-libs to the same location composer is putting it and changing the branch with git). This new subscriber would simply have the new filter logic you added for the time being.
3. Note that because each of these ultimately become their own repositories after the monorepo is split, you need to update the required dependencies in src/ACF/composer.json to add "moderntribe/square1-container": "^3.5", as a subscriber relies on the Abstract_Subscriber.
4. Your your square one PR would need to be updated to point to the newTribe\Libs\ACF\Block_Category::CUSTOM_BLOCK_CATEGORY_SLUG constant from this repo in all the blocks (sorry).
5. Once we release this new tribe-libs version, update your square one PR via a composer update moderntribe/tribe-libs and new forks will work out of the box.

Lots to digest here, I know, but I wanted a shareable write up here instead of just PRing fixes but it got a little out of hand 😊

Happy to chat further or provide any help needed.

[defunctl]

This is missing the use Tribe\Project\Blocks\Block_Category; at the top of the file.

We wouldn't add this exact namespace if we make any of the suggested changes above.

[defunctl]

@ChrisMKindred FYI I ended up in a similar situation where a change to a template would break existing projects, so I added a second template and a check to determine which one to use:

tribe-libs/src/Generators/Block_Generator.php

Lines 106 to 110 in 64bf5bd

	// Check for updated Base_Model method for versions of square-one that have block middleware.
	// @phpstan-ignore-next-line
	$template_path = method_exists( '\Tribe\Project\Blocks\Types\Base_Model', 'init_data' )
	? __DIR__ . '/templates/block/model-35.php.tmpl' // tribe libs 3.5 version
	: __DIR__ . '/templates/block/model.php.tmpl'; // tribe libs 3.4.x version

[ChrisMKindred]

I am going to move over the Block_Category class from SquareOne so that the const is available.

I cleaned up my PR so that we are calling a valid category and referencing the Block_Category::CUSTOM_BLOCK_CATEGORY_SLUG in the TODO the same way we are for the SVG and Description. I completely agree with the suggestion to move Block_Cateogry over and created an issue (#124) referencing the above review to be addressed in the future.

[ChrisMKindred]

closed in favor of #127