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
405 stars 218 forks source link

Product Elements: Add support for fluid type #8152

Closed sunyatasattva closed 1 year ago

sunyatasattva commented 1 year ago

All Product Elements blocks should support fluid type where possible, so themes that use it work more optimally on smaller scales.

danieldudzic commented 1 year ago

My understanding of "adding support" for fluid type is basically not breaking the inheritance of typography settings (font-size, line-height). Please correct me if I'm wrong.

I have done a significant amount of testing today with product elements and fluid typography themes, and it seems that product elements already support fluid typography quite well.

JppCgWtagI

sunyatasattva commented 1 year ago

It seems they do. This issue was supposed to be also a QA/research spike, as many of the issues currently in this project. If you are confident in your tests, we can close the issue as completed.

I don't know if there is anything we should do this compatibility is not broken, though. For instance, we could do component testing, for example using Storybook. What do you think?

danieldudzic commented 1 year ago

Oh sorry forgot to mention: Important note, the above gif displays a scenario in which fluid typo theme provides custom typography setting for our blocks. I have noticed that some of the blocks don't really scale out of the box (without custom settings coming from the theme).

For instance, we could do component testing, for example using Storybook. What do you think?

We could, but to be honest, this doesn't feel like a good use of our time. As long as we use ems and rems, we should be good.

Perhaps, we can look into improving (our) default styling and include fluid typography to improve the behavior across all viewport sizes out of the box (not relying on themes to provide custom styles)?

danieldudzic commented 1 year ago

This is kind of a similar issue to revamping Product Elements margins. I'd appreciate your input @vivialice - would you have any design suggestions for fluid typography default setup (font-sizes, line-heights, etc.) for Product Elements?

kmanijak commented 1 year ago

would you have any design suggestions for fluid typography default setup (font-sizes, line-heights, etc.) for Product Elements?

I think this is the kind of issue in which we could probably suggest some initial values, potentially based on a font-size mixin to unblock the issue.

cc: @tjcafferkey, @imanish003 - the issue needs design. But as we discussed elsewhere, I think we can go ahead and propose something, what's your opinion? The "before" and "after" can be then presented for a sign-off or feedback.

imanish003 commented 1 year ago

@kmanijak That sounds good to me. We should not block ourselves from completing the task. @tjcafferkey What do you think? 🙂

tjcafferkey commented 1 year ago

@kmanijak @imanish003 if we can proceed with the proposal assuming we're not blocked, then we can always get Jarek to sign it off if required. Yep!

imanish003 commented 1 year ago

Thanks @tjcafferkey 🙌🏻 @kmanijak As we discussed, I have added 🔨 emoji with this task to mark it ready to work on 🙂

kmanijak commented 1 year ago

I checked the following blocks:

Each of them has a default font-size and line-height expressed in rem or em and uses global variables to define their default size.

To make this comment easier to read I'll reference just font-size but I mean both properties: font-size and line-height.

Then following Danny's notes:

Important note, the above gif displays a scenario in which a fluid typo theme provides a custom typography setting for our blocks. I have noticed that some of the blocks don't really scale out of the box (without custom settings coming from the theme).

I double checked and that behavior can be seen:

And I believe that's correct and expected behavior. Block is prepared to be responsive and uses relative font size, but the theme controls it. So my suggestion is not to change this behavior and not add scaling out of the box. Reasoning:

I'm open to hearing otherwise, so please comment to provide a different view or if you can spot other work that could/should be done in the scope of this issue. If you agree or there are no comments I'll go ahead and close this issue in 2 days - Friday, March 24 (I hope that's enough time for potential feedback). 🙌

imanish003 commented 1 year ago

Hi @kmanijak,

Thank you for the detailed analysis and for considering the feedback from Danny. 🙏🏻 It's helpful to know that the font sizes and line heights for each of the blocks have default values expressed in rem or em and use global variables to define their default size. It's also good to hear that the behavior of the blocks is expected and consistent with the behavior of other core blocks like Site Tagline or Post Content.

I agree with your suggestion to leave the control of font scaling to the theme and not add scaling out of the box. 🤝 It seems like the easiest and most logical approach, and it would avoid unnecessary complexity.

Thanks again for your hard work on this issue. If I have any more feedback or suggestions, I'll make sure to let you know before the deadline. 🙌

kmanijak commented 1 year ago

Thanks @imanish003 for your response. Given the result of my research and your confirmation, I'm closing this issue and considering it done ✅

But the closed issues can still be commented on in case someone has anything to add to this thread 🙌