Improve performance and simplify usage of block_has_support() by supporting a string

[kt-12]

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

---

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.

[mukeshpanchal27]

Thanks @kt-12 for the PR. Solid start. Left few suggestion.

One instance is missing here https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/block-supports/border.php#L25 that need to update as well. Thanks.

[mukeshpanchal27]

Suggested change

	} else {
	$block_support = isset( $block_type->supports[ $feature ] ) ? $block_type->supports[ $feature ] : $default_value;
	}
	} elseif ( isset( $block_type->supports[ $feature ] ) ) {
	$block_support = $block_type->supports[ $feature ];
	}

The default value already set at starting of the function so i will use elseif.

[felixarntz]

We should also add here another condition based on @swissspidy's feedback in https://core.trac.wordpress.org/ticket/58532#comment:4.

[mukeshpanchal27]

Instead of having two separate tests, I will use a single test with a data provider to test different scenarios. This approach allows me to efficiently test multiple cases using different data inputs within a single test method.

	/**
	 * Data provider for test_block_has_support_string
	 */
	public function data_block_has_support_string() {
		return array(
			array(
				array(),
				'color',
				false,
			),
			array(
				array(
					'supports' => array(
						'align'    => array( 'wide', 'full' ),
						'fontSize' => true,
						'color'    => array(
							'link'     => true,
							'gradient' => false,
						),
					),
				),
				'align',
				true,
			),
			array(
				array(
					'supports' => array(
						'align'    => array( 'wide', 'full' ),
						'fontSize' => true,
						'color'    => array(
							'link'     => true,
							'gradient' => false,
						),
					),
				),
				'fontSize',
				true,
			),
			array(
				array(
					'supports' => array(
						'align'    => array( 'wide', 'full' ),
						'fontSize' => false,
						'color'    => array(
							'link'     => true,
							'gradient' => false,
						),
					),
				),
				'fontSize',
				false,
			),
			array(
				array(
					'supports' => array(
						'align'    => array( 'wide', 'full' ),
						'fontSize' => true,
						'color'    => array(
							'link'     => true,
							'gradient' => false,
						),
					),
				),
				'anchor',
				false,
			),
		);
	}

	/**
	 * @ticket 58532
	 *
	 * @dataProvider data_block_has_support_string
	 *
	 * @param array  $block_data Block data.
	 * @param string $support    Support string to check.
	 * @param bool   $expected   Expected result.
	 */
	public function test_block_has_support_string( $block_data, $support, $expected ) {
		$this->registry->register( 'core/example', $block_data );
		$block_type  = $this->registry->get_registered( 'core/example' );
		$has_support = block_has_support( $block_type, $support );
		$this->assertEquals( $expected, $has_support );
	}

[kt-12]

Thanks @mukeshpanchal27. This make sense, I have updated the test cases.

[swissspidy]

I think we should also check the length of the array, e.g. something like

if ( is_array( $feature ) && count( $feature ) === 1 ) {
	$feature = $feature[0];
}

If someone passes an array with just 1 element in it we can also short-circuit.

This way we don't necessarily have to change all existing instances in core, but can keep them as is.

And plugins and themes doing it wrong also benefit.

[felixarntz]

+1 to this change, that said I still think it's worth changing the instances in core since it's IMO just a bit simpler to use strings, more intuitive, and a tiny bit faster to check for.

[kt-12]

I have included this check. Did a small rearrangement of condition to include this. w.r.t to core I have updated all the single element array to string.

[felixarntz]

Thank you @kt-12, this looks great. Only two very minor docs suggestions other than the feedback that was already provided.

[felixarntz]

We should also add here another condition based on @swissspidy's feedback in https://core.trac.wordpress.org/ticket/58532#comment:4.

[felixarntz]

+1 to this change, that said I still think it's worth changing the instances in core since it's IMO just a bit simpler to use strings, more intuitive, and a tiny bit faster to check for.

[felixarntz]

Thanks @kt-12, LGTM!

[mukeshpanchal27]

Great work @kt-12,

One instance is missing here https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/block-supports/border.php#L25 that need to update as well. Thanks.

[felixarntz]

Committed in https://core.trac.wordpress.org/changeset/56382