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

Product Rating: Transition from using CSS margin to Global Styles #8202

Merged
merged 7 commits into from
Jan 20, 2023

Conversation

danieldudzic
Copy link
Contributor

@danieldudzic danieldudzic commented Jan 16, 2023

As a part of #7954 I'm porting product elements to utilize Global Styles instead of CSS.

This PR moves the margin from CSS to Global Styles for the Product Rating element.

It also addresses the #8041 regression.

User Facing Testing

  1. Add a Products block on a new page and add the Product Rating block inside of it.
  2. Ensure the Product Rating margin is displaying correctly (and is present in the Dimensions > Margin setting).
  3. Iterate over each Products block pattern (which includes the Rating block). Make sure the margin is correct.
  4. Make sure the alignment setting works (for Product Rating) correctly in the editor and frontend.
  • Do not include in the Testing Notes

WooCommerce Visibility

  • WooCommerce Core
  • Feature plugin
  • Experimental

Performance Impact

Changelog

Enhancement: Move margin for Product Rating from CSS to Global Styles.

@danieldudzic danieldudzic added the type: enhancement The issue is a request for an enhancement. label Jan 16, 2023
@danieldudzic danieldudzic self-assigned this Jan 16, 2023
@woocommercebot woocommercebot requested review from a team and albarin and removed request for a team January 16, 2023 14:09
@github-actions
Copy link
Contributor

github-actions bot commented Jan 16, 2023

The release ZIP for this PR is accessible via:

https://wcblocks.wpcomstaging.com/wp-content/uploads/woocommerce-gutenberg-products-block-8202.zip

Script Dependencies Report

There is no changed script dependency between this branch and trunk.

This comment was automatically generated by the ./github/compare-assets action.

TypeScript Errors Report

  • Files with errors: 497
  • Total errors: 2251

🎉 🎉 This PR does not introduce new TS errors.

comments-aggregator

@github-actions
Copy link
Contributor

github-actions bot commented Jan 16, 2023

Size Change: +18 B (0%)

Total Size: 1.08 MB

Filename Size Change
build/wc-blocks-style-rtl.css 25.7 kB +9 B (0%)
build/wc-blocks-style.css 25.6 kB +9 B (0%)
ℹ️ View Unchanged
Filename Size
build/active-filters-frontend.js 7.99 kB
build/active-filters-wrapper-frontend.js 6.01 kB
build/active-filters.js 7.31 kB
build/all-products-frontend.js 11.6 kB
build/all-products.js 33.5 kB
build/all-reviews.js 7.67 kB
build/attribute-filter-frontend.js 22.9 kB
build/attribute-filter-wrapper-frontend.js 7.68 kB
build/attribute-filter.js 12.4 kB
build/blocks-checkout.js 41 kB
build/cart-blocks/cart-accepted-payment-methods-frontend.js 1.38 kB
build/cart-blocks/cart-cross-sells-frontend.js 253 B
build/cart-blocks/cart-cross-sells-products-frontend.js 9.64 kB
build/cart-blocks/cart-express-payment--checkout-blocks/express-payment-frontend.js 5.08 kB
build/cart-blocks/cart-express-payment-frontend.js 720 B
build/cart-blocks/cart-items-frontend.js 299 B
build/cart-blocks/cart-line-items--mini-cart-contents-block/products-table-frontend.js 5.34 kB
build/cart-blocks/cart-line-items-frontend.js 1.07 kB
build/cart-blocks/cart-order-summary-frontend.js 1.25 kB
build/cart-blocks/cart-totals-frontend.js 321 B
build/cart-blocks/empty-cart-frontend.js 345 B
build/cart-blocks/filled-cart-frontend.js 656 B
build/cart-blocks/order-summary-coupon-form-frontend.js 1.69 kB
build/cart-blocks/order-summary-discount-frontend.js 2.12 kB
build/cart-blocks/order-summary-fee-frontend.js 274 B
build/cart-blocks/order-summary-heading-frontend.js 456 B
build/cart-blocks/order-summary-shipping-frontend.js 14.9 kB
build/cart-blocks/order-summary-subtotal-frontend.js 274 B
build/cart-blocks/order-summary-taxes-frontend.js 435 B
build/cart-blocks/proceed-to-checkout-frontend.js 1.24 kB
build/cart-frontend.js 28.5 kB
build/cart.js 47.5 kB
build/catalog-sorting.js 1.71 kB
build/checkout-blocks/actions-frontend.js 1.85 kB
build/checkout-blocks/billing-address--checkout-blocks/shipping-address-frontend.js 3.91 kB
build/checkout-blocks/billing-address-frontend.js 1.15 kB
build/checkout-blocks/contact-information-frontend.js 2.04 kB
build/checkout-blocks/express-payment-frontend.js 1.13 kB
build/checkout-blocks/fields-frontend.js 344 B
build/checkout-blocks/order-note-frontend.js 1.14 kB
build/checkout-blocks/order-summary-cart-items-frontend.js 3.68 kB
build/checkout-blocks/order-summary-coupon-form-frontend.js 1.85 kB
build/checkout-blocks/order-summary-discount-frontend.js 2.29 kB
build/checkout-blocks/order-summary-fee-frontend.js 277 B
build/checkout-blocks/order-summary-frontend.js 1.25 kB
build/checkout-blocks/order-summary-shipping-frontend.js 14.9 kB
build/checkout-blocks/order-summary-subtotal-frontend.js 275 B
build/checkout-blocks/order-summary-taxes-frontend.js 435 B
build/checkout-blocks/payment-frontend.js 8.32 kB
build/checkout-blocks/pickup-options-frontend.js 2.81 kB
build/checkout-blocks/shipping-address-frontend.js 1.11 kB
build/checkout-blocks/shipping-method-frontend.js 2.27 kB
build/checkout-blocks/shipping-methods-frontend.js 4.83 kB
build/checkout-blocks/terms-frontend.js 1.56 kB
build/checkout-blocks/totals-frontend.js 324 B
build/checkout-frontend.js 30.1 kB
build/checkout.js 43.3 kB
build/customer-account.js 3.08 kB
build/featured-category.js 13.1 kB
build/featured-product.js 13.4 kB
build/filter-wrapper-frontend.js 14.1 kB
build/filter-wrapper.js 2.4 kB
build/general-style-rtl.css 1.31 kB
build/general-style.css 1.31 kB
build/handpicked-products.js 7.24 kB
build/legacy-template.js 2.87 kB
build/mini-cart-component-frontend.js 27.7 kB
build/mini-cart-contents-block/empty-cart-frontend.js 366 B
build/mini-cart-contents-block/filled-cart-frontend.js 268 B
build/mini-cart-contents-block/footer-frontend.js 2.82 kB
build/mini-cart-contents-block/items-frontend.js 237 B
build/mini-cart-contents-block/products-table-frontend.js 590 B
build/mini-cart-contents-block/shopping-button-frontend.js 313 B
build/mini-cart-contents-block/title-frontend.js 367 B
build/mini-cart-contents.js 16.9 kB
build/mini-cart-frontend.js 1.88 kB
build/mini-cart.js 4.29 kB
build/price-filter-frontend.js 13.9 kB
build/price-filter-wrapper-frontend.js 7.01 kB
build/price-filter.js 8.4 kB
build/price-format.js 1.19 kB
build/product-add-to-cart--product-button--product-category-list--product-image--product-price--product-r--a0326d00.js 226 B
build/product-add-to-cart--product-button--product-image--product-rating--product-title.js 151 B
build/product-add-to-cart-frontend.js 6.71 kB
build/product-add-to-cart.js 8.47 kB
build/product-best-sellers.js 7.61 kB
build/product-button--product-category-list--product-image--product-price--product-rating--product-sale-b--e17c7c01.js 439 B
build/product-button--product-image--product-price--product-rating--product-sale-badge--product-title.js 257 B
build/product-button-frontend.js 2.14 kB
build/product-button.js 3.84 kB
build/product-categories.js 2.36 kB
build/product-category-list-frontend.js 1.14 kB
build/product-category-list.js 503 B
build/product-category.js 8.6 kB
build/product-image-frontend.js 2.14 kB
build/product-image.js 3.94 kB
build/product-new.js 7.6 kB
build/product-on-sale.js 7.93 kB
build/product-price-frontend.js 2.23 kB
build/product-price.js 1.58 kB
build/product-query.js 5.97 kB
build/product-rating-frontend.js 1.57 kB
build/product-rating.js 920 B
build/product-results-count.js 1.68 kB
build/product-sale-badge-frontend.js 1.37 kB
build/product-sale-badge.js 814 B
build/product-search.js 2.61 kB
build/product-sku-frontend.js 629 B
build/product-sku.js 377 B
build/product-stock-indicator-frontend.js 1.26 kB
build/product-stock-indicator.js 645 B
build/product-summary-frontend.js 1.53 kB
build/product-summary.js 920 B
build/product-tag-list-frontend.js 1.13 kB
build/product-tag-list.js 496 B
build/product-tag.js 8.08 kB
build/product-title-frontend.js 1.57 kB
build/product-title.js 3.31 kB
build/product-top-rated.js 7.84 kB
build/products-by-attribute.js 8.52 kB
build/rating-filter-frontend.js 21.4 kB
build/rating-filter-wrapper-frontend.js 6.21 kB
build/rating-filter.js 7.44 kB
build/reviews-by-category.js 11.2 kB
build/reviews-by-product.js 12.3 kB
build/reviews-frontend.js 7.14 kB
build/single-product-frontend.js 17.7 kB
build/single-product.js 9.99 kB
build/stock-filter-frontend.js 21.1 kB
build/stock-filter-wrapper-frontend.js 5.87 kB
build/stock-filter.js 8.17 kB
build/store-notices.js 1.64 kB
build/vendors--attribute-filter-wrapper--cart-blocks/cart-cross-sells-products--cart-blocks/order-summary--82e4ed06-frontend.js 6.86 kB
build/vendors--attribute-filter-wrapper--rating-filter-wrapper--stock-filter-wrapper-frontend.js 7.7 kB
build/vendors--cart-blocks/cart-cross-sells-products--cart-blocks/cart-line-items--cart-blocks/cart-order--3c5fe802-frontend.js 5.26 kB
build/vendors--cart-blocks/cart-cross-sells-products--cart-blocks/order-summary-shipping--checkout-blocks--18f9376a-frontend.js 19.4 kB
build/vendors--cart-blocks/cart-cross-sells-products--product-add-to-cart-frontend.js 7.53 kB
build/vendors--cart-blocks/cart-line-items--checkout-blocks/order-summary-cart-items--mini-cart-contents---233ab542-frontend.js 3.14 kB
build/vendors--cart-blocks/order-summary-shipping--checkout-blocks/billing-address--checkout-blocks/order--5b8feb0b-frontend.js 4.83 kB
build/vendors--checkout-blocks/shipping-method-frontend.js 12 kB
build/vendors--checkout-blocks/shipping-methods-frontend.js 9.48 kB
build/wc-blocks-data.js 21.6 kB
build/wc-blocks-editor-style-rtl.css 5.41 kB
build/wc-blocks-editor-style.css 5.41 kB
build/wc-blocks-google-analytics.js 1.56 kB
build/wc-blocks-middleware.js 933 B
build/wc-blocks-registry.js 3.16 kB
build/wc-blocks-shared-context.js 1.52 kB
build/wc-blocks-shared-hocs.js 1.73 kB
build/wc-blocks-vendors-style-rtl.css 1.96 kB
build/wc-blocks-vendors-style.css 1.96 kB
build/wc-blocks-vendors.js 64.2 kB
build/wc-blocks.js 2.63 kB
build/wc-payment-method-bacs.js 816 B
build/wc-payment-method-cheque.js 811 B
build/wc-payment-method-cod.js 909 B
build/wc-payment-method-paypal.js 837 B
build/wc-settings.js 2.6 kB
build/wc-shipping-method-pickup-location.js 29.7 kB

compressed-size-action

@danieldudzic
Copy link
Contributor Author

@Aljullu I have noticed that (when testing with default themes) the "1" measure of margin seems to be a bit excessive visually:

Edit_Page_“New_shop”_‹ratings—_WordPress

I'd like to avoid us hardcoding anything in px or fraction em/rem values, and stick to presets as that provides the most straight-forward experience in my opinion. Do you have any suggestions?

@albarin
Copy link
Contributor

albarin commented Jan 17, 2023

Hey @danieldudzic I'm seeing some weird behaviour while testing this, take a look at the video👇 seems like the align button on the toolbar is duplicating.

CleanShot.2023-01-17.at.14.37.26.mp4

@danieldudzic
Copy link
Contributor Author

Hey @danieldudzic I'm seeing some weird behaviour while testing this, take a look at the video👇 seems like the align button on the toolbar is duplicating.
CleanShot.2023-01-17.at.14.37.26.mp4

Really weird! Good catch. I'll look into it.

@sunyatasattva
Copy link
Contributor

I'd like to avoid us hardcoding anything in px or fraction em/rem values, and stick to presets as that provides the most straight-forward experience in my opinion. Do you have any suggestions?

What is the measurement unit of this 1? Is it em ? In this case, I think it's actually sensible to add a base unit to the Reviews container element that can be increased and decreased by this control. We might need the help of @vivialice to define such sensible base unit.

If that's rem, then I'm not sure.

@danieldudzic
Copy link
Contributor Author

Hey @danieldudzic I'm seeing some weird behaviour while testing this, take a look at the video👇 seems like the align button on the toolbar is duplicating.
CleanShot.2023-01-17.at.14.37.26.mp4

@albarin I'm struggling to replicate this. Do you see the same issue in trunk?

@albarin
Copy link
Contributor

albarin commented Jan 18, 2023

@albarin I'm struggling to replicate this. Do you see the same issue in trunk?

Sorry @danieldudzic, not sure what happened yesterday while testing, but I just tried trunk and also your branch and it's not happening anymore. Approving!

@github-actions github-actions bot added this to the 9.5.0 milestone Jan 18, 2023
@danieldudzic
Copy link
Contributor Author

I'd like to avoid us hardcoding anything in px or fraction em/rem values, and stick to presets as that provides the most straight-forward experience in my opinion. Do you have any suggestions?

What is the measurement unit of this 1? Is it em ? In this case, I think it's actually sensible to add a base unit to the Reviews container element that can be increased and decreased by this control. We might need the help of @vivialice to define such sensible base unit.

If that's rem, then I'm not sure.

The scale can be defined via theme.json.

Here are the defaults:

CSS custom propertyValueEditor label
–wp–preset–spacing–200.44rem2X-Small
–wp–preset–spacing–300.67remX-Small
–wp–preset–spacing–401remSmall
–wp–preset–spacing–501.5remMedium
–wp–preset–spacing–602.25remLarge
–wp–preset–spacing–703.38remX-Large
–wp–preset–spacing–805.06rem2X-Large

With 1 using –wp–preset–spacing–30. The 2022 theme doesn't set a custom scale so, 1 defaults to 0.67rem .

In case of 2023 theme, it sets a custom scale, and 1 is set to clamp(1.5rem, 5vw, 2rem)).

@danieldudzic
Copy link
Contributor Author

@sunyatasattva We have discussed this, with @Aljullu and agreed that it's not the worst solution to set an arbitrary em/rem value as the Global Style margin, since those presets (like –wp–preset–spacing–30) make much more sense when it comes to distances between blocks (and not necessarily inner blocks).

Regarding your suggestion:

I think it's actually sensible to add a base unit to the Reviews container element that can be increased and decreased by this control. We might need the help of @vivialice to define such sensible base unit.

What you are suggesting is setting a value for the Rating container that would affect the scale on the inner block level. Is that correct?

I believe @nerrad suggested something similar regarding font sizes for Products patterns inner blocks, which would help with the responsiveness of sizes for various viewports.

@danieldudzic
Copy link
Contributor Author

I have re-added CSS margins for the All Products block context, so as to not visually break it for existing instances of the All Products block.

Copy link
Contributor

@Aljullu Aljullu left a comment

Choose a reason for hiding this comment

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

Looks good to me, @danieldudzic! I tested several patterns and all of them seem to look good, I also tested the All Products block, PHP-based grid blocks and classic templates to make sure there are no regressions and all of them look good.

I just left a question below about a code snippet which I'm not sure to understand, but besides that, LGTM.

assets/js/atomic/blocks/product-elements/rating/style.scss Outdated Show resolved Hide resolved
@danieldudzic
Copy link
Contributor Author

Failing tests seem to be unrelated to the committed code. I will go ahead and merge this.

@danieldudzic danieldudzic merged commit 18c5591 into trunk Jan 20, 2023
@danieldudzic danieldudzic deleted the update/7954-product-rating-margin branch January 20, 2023 15:04
danieldudzic added a commit that referenced this pull request Feb 1, 2023
nielslange added a commit that referenced this pull request Feb 1, 2023
* Empty commit for release pull request

* Update readme.txt

* Bump “minimum_supported_wp_version” in phpcs.xml

* Add testing notes

* Update 950.md

* Update 950.md

* Update 950.md

* Update 950.md

* Update 950.md

* Update 950.md

* Update #8202 testing instructions.

* Revert "Set inherit default to true only when is inserted in the archive product template (#8251)" (#8352)

This reverts commit 39b0f91.

* Update release ZIP file

* Bumping version strings to new version.

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Niels Lange <info@nielslange.de>
Co-authored-by: Daniel Dudzic <daniel.dudzic@automattic.com>
Co-authored-by: Alba Rincón <albarin@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: enhancement The issue is a request for an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants