@rrennick: For context, whenever you're reviewing this, we had a little discussion in Slack on whether this was a breaking change or not, and we think it's ok provided the template version is bumped, etc. Here's the link for your reading pleasure: https://woocommercecommunity.slack.com/archives/C052LGBQD1T/p1681941407318249.

Codecov Report

Merging #37853 (924c871) into trunk (c5285ec) will increase coverage by 0.0%.
The diff coverage is 100.0%.

❗ Current head 924c871 differs from pull request most recent head 1fa2812. Consider uploading reports for the commit 1fa2812 to get more accurate results

@@           Coverage Diff            @@
##             trunk   #37853   +/-   ##
========================================
  Coverage     51.5%    51.5%           
+ Complexity   17283    17281    -2     
========================================
  Files          430      430           
  Lines        80037    80030    -7     
========================================
+ Hits         41216    41217    +1     
+ Misses       38821    38813    -8     

@dkotter Thanks for submitting this. You will have to check the SCSS for remaining 4 default themes for styles for the <figure>. In Twenty-twenty it's https://github.com/woocommerce/woocommerce/blob/trunk/plugins/woocommerce/client/legacy/css/twenty-twenty.scss#L526

@dkotter Thanks for submitting this. You will have to check the SCSS for remaining 4 default themes for styles for the <figure>. In Twenty-twenty it's https://github.com/woocommerce/woocommerce/blob/trunk/plugins/woocommerce/client/legacy/css/twenty-twenty.scss#L526

@rrennick This is already accounted for in each of those files (see https://github.com/woocommerce/woocommerce/blob/trunk/plugins/woocommerce/client/legacy/css/twenty-twenty.scss#L531 for one example). They have separate CSS for the figure tag and then right below that, they have CSS for the woocommerce-product-gallery__wrapper class, which is the class that is used on that figure tag (and now will be used on the div tag).

I think we want to leave the existing figure CSS to support the use case of sites that have overridden the single product image template and thus will still have the figure HTML

I think we want to leave the existing figure CSS to support the use case of sites that have overridden the single product image template and thus will still have the figure HTML

While I see where you are coming from, that isn't consistent with the changes to the Twenty-Twenty-Two/Three CSS. We should definitely be consistent across all the default themes.

From Slack discussion: The

>

could be considered a breaking change, but I’m not sure if we really enforce that in templates, other than bumping template version number.

WooCommerce adds a warning to both the WooCommerce > Status screen and the Plugins screen when a template is out of date. We do make changes to templates that are not 100% backward compatible.

While I see where you are coming from, that isn't consistent with the changes to the Twenty-Twenty-Two/Three CSS. We should definitely be consistent across all the default themes

I guess I'm not following what isn't consistent here? Happy to make any changes as needed but not exactly sure what you're referring to.

In our CSS for Twenty-Twenty-Two/Three, the existing code is:

figure.woocommerce-product-gallery__wrapper {
	margin: 0;
}

I've left the figure part but added div.woocommerce-product-gallery__wrapper to that. I can remove the figure if you think that is no longer needed.

The CSS for all other Twenty themes has these lines:

figure {
    margin: 0;
    padding: 0;
}

.woocommerce-product-gallery__wrapper {
    margin: 0;
    padding: 0;
}

While yes, this isn't the exact same as what is done in Twenty-Twenty-Two/Three, the end result in essence is the same (we target both the figure tag and any use of the woocommerce-product-gallery__wrapper class). If the idea is to try and make these lines as consistent as possible across all default themes, I can look to do that, was just trying to keep this PR as focused as possible and minimize risk of breaking styles.