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

Product Elements: Add support for fluid type #8152

Closed
Tracked by #42616
sunyatasattva opened this issue Jan 11, 2023 · 11 comments
Closed
Tracked by #42616

Product Elements: Add support for fluid type #8152

sunyatasattva opened this issue Jan 11, 2023 · 11 comments
Assignees
Labels
block-type: product elements Issues related to Product Element blocks. focus: global styles Issues that involve styles/css/layout structure. type: enhancement The issue is a request for an enhancement.

Comments

@sunyatasattva
Copy link
Contributor

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

@sunyatasattva sunyatasattva added type: enhancement The issue is a request for an enhancement. focus: global styles Issues that involve styles/css/layout structure. block-type: product elements Issues related to Product Element blocks. labels Jan 11, 2023
@danieldudzic danieldudzic self-assigned this Jan 11, 2023
@danieldudzic
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

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?

@sunyatasattva sunyatasattva added the needs: design The issue requires design input/work from a designer. label Feb 16, 2023
@kmanijak
Copy link
Contributor

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
Copy link
Contributor

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

@tjcafferkey
Copy link
Contributor

@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 imanish003 removed the needs: design The issue requires design input/work from a designer. label Mar 15, 2023
@imanish003
Copy link
Contributor

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

@kmanijak kmanijak assigned kmanijak and unassigned danieldudzic Mar 21, 2023
@kmanijak
Copy link
Contributor

kmanijak commented Mar 22, 2023

I checked the following blocks:

  • Product Image
  • Product Title
  • Product Price
  • Product SKU
  • Product Summary
  • Product Rating
  • Product Stock Indicator
  • On sale badge
  • Add to Cart Button

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:

  • TT2 - body's font-size is static, hence the relative (rem) font-sizes of the blocks don't change with the width of the screen
  • TT3 - body's font-size is dynamic, hence the relative (rem) font-sizes of the blocks change in some range

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 checked Core blocks like Site Tagline or Post Content and they behave the same (have relative font size, but don't scale by themselves. They leave the control to the theme)
  • It's easy to leave the control to the theme to ENABLE scaling but controlling just a single font-size property on a body element, but it would be unnecessarily complex to force them to DISABLE the out-of-the-box scaling

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
Copy link
Contributor

imanish003 commented Mar 23, 2023

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
Copy link
Contributor

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 🙌

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
block-type: product elements Issues related to Product Element blocks. focus: global styles Issues that involve styles/css/layout structure. type: enhancement The issue is a request for an enhancement.
Projects
None yet
Development

No branches or pull requests

5 participants