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

Product grid Blocks have some differences compared to product list #3349

Closed
MaggieCabrera opened this issue Oct 30, 2020 · 4 comments
Closed
Labels
type: bug The issue/PR concerns a confirmed bug.

Comments

@MaggieCabrera
Copy link

Describe the bug

  • The blocks don't have a border around the product image
  • The "Sale" badge is positioned differently on the blocks compared to the list
  • The old price (before discount) text has opacity 0.5 on the product list page but not on the blocks.

To reproduce

Steps to reproduce the behavior:

  • With Varia theme on, install one of the blocks that shows a grid of products such as Best Selling Products
  • Compare the appearance to the list of products page

Expected behavior

I expected both views to look the same

Screenshots

Product list page:

Screenshot 2020-10-30 at 17 12 04

Best Selling Products Block

Screenshot 2020-10-30 at 16 43 26

I added a background to the body in order to see the position of the sale badge:

Screenshot 2020-10-30 at 17 11 54

Enviroment

WordPress (please complete the following information):

  • Core version: 5.5.2
  • Gutenberg version (if installed): 9.2.1
  • WooCommerce version: 4.6.1
  • WooCommerce Blocks version: 3.7.0

Desktop (please complete the following information):

  • OS: macOS 10.15.7
  • Browser: FF
  • Version: 82

I wonder if these blocks and the regular grid should share some css/classes inorder to avoid duplicate css both on Woo's side and on the theme's. Right now it looks like any change that we want to do to the style of the products grids will need to be done twice.

@MaggieCabrera MaggieCabrera added the type: bug The issue/PR concerns a confirmed bug. label Oct 30, 2020
@nerrad
Copy link
Contributor

nerrad commented Oct 30, 2020

Hi @MaggieCabrer👋🏻,

I'll let @Aljullu weigh in on the inconsistencies with the sales badge positioning between the different contexts, that may be something we can look into addressing.

Besides that I'd like to address this:

I wonder if these blocks and the regular grid should share some css/classes inorder to avoid duplicate css both on Woo's side and on the theme's. Right now it looks like any change that we want to do to the style of the products grids will need to be done twice.

To give some context around why things are the way they are now, it's good to review #2403 and #891. Besides the rationale given in those two issues (which is important and I won't rehash here), we have intentionally departed from consistency with structure and css between the two views because:

  • Eventually WooCommerce blocks, block patterns, and Woo specific block templates will replace existing archive views. It is better in the long run to have a clear demarcation between the different rendered views.
  • Many of the SSR blocks will be refactored to be containers around product element blocks. Since there's a possibility some css/structure could change as a part of that refactor, we want to limit the code churn as much as possible.

@Aljullu
Copy link
Contributor

Aljullu commented Nov 2, 2020

Thanks for opening this issue @MaggieCabrera, loads of good stuff here!

The blocks don't have a border around the product image

Nice catch with this. I'm pretty sure that was unnoticed by us because it can't be reproduced in WC default theme (Storefront). I created an issue to fix this (or leave it unfixed, depending on design feedback): #3354.

The "Sale" badge is positioned differently on the blocks compared to the list

That seems to be because Varia adds some specific styles to the sale badge. This is what I see when I inspect it:

#content .wc-block-grid .wc-block-grid__product .wc-block-grid__product-onsale {
    right: calc(-0.5em + 16px);
    ...
}

Should we open an issue in Varia's repo in order to investigate that? I'm not sure about the purpose of those styles.

The old price (before discount) text has opacity 0.5 on the product list page but not on the blocks.

It's true there is an inconsistency with WooCommerce Core styles here. But in this case I don't think we want to fix it, we are trying as much as possible to avoid changing the opacity of text for accessibility reasons. That can't be undone in WC Core without breaking the styles from live stores, but WC Blocks allows us to "start from scratch" in this field, and I think it's the right decision not to tweak the opacity (but I'm happy to discuss 🙂 ).

My suggestion would be to implement the desired style in the theme. I see Varia has this selector:

#woocommerce-wrapper ul.products li.product .price del ... {
  opacity: 0.5;
  ...
}

You could add the WC Blocks specific selector to it and all blocks will inherit those styles too:

#woocommerce-wrapper ul.products li.product .price del,
+.wc-block-components-product-price__regular {
  opacity: 0.5;
  ...
}

I don't know if you are working on Varia. If not, let me know and I can create the issues in the repo. 🙂

@MaggieCabrera
Copy link
Author

Thanks for your feedback both of you!

Besides the rationale given in those two issues (which is important and I won't rehash here), we have intentionally departed from consistency with structure and css between the two views because:

@nerrad I completely understand what you mean and makes a lot of sense

Nice catch with this. I'm pretty sure that was unnoticed by us because it can't be reproduced in WC default theme (Storefront).

@Aljullu thanks for pointing this out! I thought the discrepancy was coming from the block's css but I wasn't looking at it correctly

I chose Varia as an example because it had minimal styles but my point with this issue was trying to streamline how the block is going to look right at its base (rather than making these tweaks for every single theme we design and develop) and leave to the themes just customization, not homogenization between blocks and the rest of the store. I see this will not be an issue in the future for the reasons @nerrad explained so feel free to close if you feel like this issue is not relevant anymore.

Thank you for your time!

@nerrad
Copy link
Contributor

nerrad commented Nov 3, 2020

Thanks Maggie for raising the issues - it looks like anything actionable from this issue has been noted so I'll go ahead and close it.

@nerrad nerrad closed this as completed Nov 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug The issue/PR concerns a confirmed bug.
Projects
None yet
Development

No branches or pull requests

3 participants