Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Add wide and full alignment support for legacy template block #5433

Merged
merged 13 commits into from
Jan 7, 2022
Merged

Add wide and full alignment support for legacy template block #5433

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
fe21f86
add align wide and full support for legacy template block
gigitux Dec 21, 2021
7a755d8
fix PHP warning
gigitux Dec 22, 2021
bb19d24
add a comment on get_markup_with_classes_by_attributes
gigitux Dec 23, 2021
6166306
rename function
gigitux Dec 27, 2021
012741e
add align wide and full support for legacy template block
gigitux Dec 21, 2021
987445e
fix PHP warning
gigitux Dec 22, 2021
187bb61
add a comment on get_markup_with_classes_by_attributes
gigitux Dec 23, 2021
b4cf934
rename function
gigitux Dec 27, 2021
2aeca80
Merge branch 'add/5229-add-align-support-legacy-template' of github.c…
gigitux Jan 4, 2022
f4689e6
fix regex
gigitux Jan 4, 2022
e291dc3
update regex
gigitux Jan 5, 2022
891edf7
update regex
gigitux Jan 6, 2022
6ed25c3
fix code styling
gigitux Jan 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion assets/js/blocks/legacy-template/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ registerBlockType( 'woocommerce/legacy-template', {
'woo-gutenberg-products-block'
),
supports: {
align: false,
align: [ 'wide', 'full' ],
html: false,
multiple: false,
reusable: false,
Expand Down
53 changes: 53 additions & 0 deletions src/BlockTypes/LegacyTemplate.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
<?php
namespace Automattic\WooCommerce\Blocks\BlockTypes;

use Automattic\WooCommerce\Blocks\Utils\StyleAttributesUtils;

/**
* Legacy Single Product class
*
Expand All @@ -21,6 +23,14 @@ class LegacyTemplate extends AbstractDynamicBlock {
*/
protected $api_version = '2';

/**
* Initialize this block.
*/
protected function initialize() {
parent::initialize();
add_filter( 'render_block', array( $this, 'add_alignment_class_to_wrapper' ), 10, 2 );
}

/**
* Render method for the Legacy Template block. This method will determine which template to render.
*
Expand Down Expand Up @@ -180,4 +190,47 @@ protected function render_archive_product() {
wp_reset_postdata();
return ob_get_clean();
}

/**
* Get HTML markup with the right classes by attributes.
* This function appends the classname at the first element that have the class attribute.
* Based on the experience, all the wrapper elements have a class attribute.
*
* @param string $content Block content.
* @param array $block Parsed block data.
* @return string Rendered block type output.
*/
public function add_alignment_class_to_wrapper( string $content, array $block ) {
if ( ( 'woocommerce/' . $this->block_name ) !== $block['blockName'] ) {
return $content;
}

$attributes = (array) $block['attrs'];
$align_class_and_style = StyleAttributesUtils::get_align_class_and_style( $attributes );

if ( ! isset( $align_class_and_style['class'] ) ) {
return $content;
}

// Find the first tag.
$first_tag = '<[^<>]+>';
$matches = array();
preg_match( $first_tag, $content, $matches );

// If there is a tag, but it doesn't have a class attribute, add the class attribute.
if ( isset( $matches[0] ) && strpos( $matches[0], 'class' ) === false ) {
Copy link
Member

Choose a reason for hiding this comment

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

We should check for the exact class attribute here. Elements can have class as a part of attribute name or attribute value, like: data-class, data-type="class", id="fist-class".

Suggested change
if ( isset( $matches[0] ) && strpos( $matches[0], 'class' ) === false ) {
if ( isset( $matches[0] ) && strpos( $matches[0], ' class=' ) === false ) {

$pattern_before_tag_closing = '/.+?(?=>)/';
$matches = array();
preg_match( $pattern_before_tag_closing, $content, $matches );
return preg_replace( $pattern_before_tag_closing, $matches[0] . ' class="' . $align_class_and_style['class'] . '"', $content, 1 );
Copy link
Member

Choose a reason for hiding this comment

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

Can we use backreferences in preg_replace? If doing so, we can remove the preg_match above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think with the current regex.
Because the value of match[0] is a tag element without < and >, for example:

div data-block-name="woocommerce/legacy-template" data-template="taxonomy-product_cat" id="primary"

But the regex /.+?(?=>)/ match the text before a tag closing >.

Potentially we can use str_replace, in this way:

return str_replace( $matches[0], $matches[0] . ' class="' . $align_class_and_style['class'] . '"', $content );

instead to use regex.

I don't have a strong opinion, what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Because the value of match[0] is a tag element without < and >, for example:

@gigitux I think we can use the fourth argument of preg_replace to only replace one time, you can check my example using your regex below:

<?php
$re = '/.+?(?=>)/m';
$str = '<div data-block-name="woocommerce/legacy-template" data-template="taxonomy-product_cat" id="primary" ><div class="inner">';
$subst = '$0 class="alignwide"';

$result = preg_replace($re, $subst, $str, 1);

echo "The result of the substitution is <pre>" . htmlspecialchars( $result ) . "</pre>";

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for sharing this!

}

// If there is a tag, and it has a class already, add the class attribute.
$pattern_get_class = '/(?<=class=\")[^"]+(?=\")/';
Copy link
Member

Choose a reason for hiding this comment

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

If the wrapper element uses single quotes for class, this regex will fail.

$matches_class = array();
preg_match( $pattern_get_class, $content, $matches_class );
return preg_replace( $pattern_get_class, $matches_class[0] . ' ' . $align_class_and_style['class'], $content, 1 );
Copy link
Member

Choose a reason for hiding this comment

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

I think we can follow the same approach as we use at line 223rd above.

}


}
53 changes: 53 additions & 0 deletions src/Utils/StyleAttributesUtils.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,59 @@ public static function get_background_color_class_and_style( $attributes ) {
return null;
}

/**
* Get class and style for align from attributes.
*
* @param array $attributes Block attributes.
*
* @return (array | null)
*/
public static function get_align_class_and_style( $attributes ) {
dinhtungdu marked this conversation as resolved.
Show resolved Hide resolved

$align_attribute = isset( $attributes['align'] ) ? $attributes['align'] : null;

if ( ! $align_attribute ) {
return null;
};

if ( 'wide' === $align_attribute ) {
return array(
'class' => 'alignwide',
'style' => null,
);
}

if ( 'full' === $align_attribute ) {
return array(
'class' => 'alignfull',
'style' => null,
);
}

if ( 'left' === $align_attribute ) {
return array(
'class' => 'alignleft',
'style' => null,
);
}

if ( 'right' === $align_attribute ) {
return array(
'class' => 'alignright',
'style' => null,
);
}

if ( 'center' === $align_attribute ) {
return array(
'class' => 'aligncenter',
'style' => null,
);
}

return null;
}

/**
* Get classes and styles from attributes.
*
Expand Down