woocommerce / woocommerce-blocks

(Deprecated) This plugin has been merged into woocommerce/woocommerce
https://wordpress.org/plugins/woo-gutenberg-products-block/
GNU General Public License v3.0
406 stars 219 forks source link

Loading placeholder colors of filter blocks and All Products block should depend on currentColor #7526

Closed Aljullu closed 2 years ago

Aljullu commented 2 years ago

Describe the bug

When loading the filter blocks and the All Products block, a loading placeholder is shown. However, it looks a bit off because it doesn't honor the font color.

To reproduce

  1. Change the font color of your theme (via Appearance > Site Editor > Styles) to something that is not black or white (ie: yellow).
  2. Add all filter blocks and the All Products block to a post or page.
  3. Visit the page in the frontend.
  4. Notice the loading placeholder look off because they don't honor the font color.

Expected behavior

Ideally we should use currentColor to style these placeholders.

Screenshots

imatge

thealexandrelara commented 2 years ago

Hey @Aljullu , I've started working on this issue but I have two questions related to it that I'd like to clarify with you before opening a PR.

  1. Should the currentColor be applied to the placeholder mixin or the ideal scenario would be to create a new mixin that uses the currentColor? If it is expected to apply the change to some specific components, which components we should apply to it?

  2. What should be the colors for the linear-gradient animation? Currently, we have the following background-image that is being animated to create the placeholder effect:

    background-color: #ebebeb !important;
    background-image: linear-gradient(90deg, #ebebeb, #f5f5f5, #ebebeb);

    As you can see, the background-color is the same as the first and the third color in the linear-gradient, however, we have a gray color as the second color. If we switch the #ebebeb color with the currentColor I think we are supposed to have a dynamic color to fill this in, do you have any idea for that?

Here is a gif showing the results of replacing #ebebeb with currentColor: Screen Capture on 2022-11-08 at 16-03-29-min

Aljullu commented 2 years ago
  1. Should the currentColor be applied to the placeholder mixin or the ideal scenario would be to create a new mixin that uses the currentColor? If it is expected to apply the change to some specific components, which components we should apply to it?

I would make the change in the mixin, so all placeholders are aligned. That might require some extra testing (including Cart and Checkout blocks) to make sure things look properly everywhere.

  1. What should be the colors for the linear-gradient animation?

We don't have any specs for that, so I would suggest trying some colors/opacities until the result looks good to you. Ideally, it shouldn't look too different from the current placeholders when the text is black/dark gray.

As you can see, the background-color is the same as the first and the third color in the linear-gradient, however, we have a gray color as the second color. If we switch the #ebebeb color with the currentColor I think we are supposed to have a dynamic color to fill this in, do you have any idea for that?

Hmmm, right, that's a tricky issue. I didn't test it, but an idea of what I would try:

  1. Create a ::before element to add a colored background with opacity. You could use background: currentColor and opacity: 0.3 or something like this. (the reason for this is to work-around the fact that there is no way to change the opacity of currentColor)
  2. In the ::after element, I would set:
background-color: transparent; /* We no longer need a background-color because it's set in ::before */
background-image: linear-gradient(90deg, transparent, currentColor, transparent);
opacity: 0.6;

Of course, the values here are just an idea, it would be good to test it with light and dark themes and try to find a combination that looks good in all of them.

thealexandrelara commented 2 years ago

@Aljullu Thank you for the suggestions and ideas, I was working on your suggestions and testing with different color combinations and in the end I think the best solution was to basically modify the linear-gradient to use the currentColor and add transparency to the middle color, this way we can have a similar loading animation effect for different colors and does not need to add a ::before element. I created a PR with the changes so it would be great to have your opinion on it: https://github.com/woocommerce/woocommerce-blocks/pull/7658