ClayCSS: Support Document Font Size to be 62.5% or 10px for easier rem/em conversion

[pat270]

Issue from Slack Chat:

Hey, a question. Is there a reason for 1rem to be 16 and not 10px in Liferay 7.1 ga1 in the Classic Theme? (i know 16px is default but isn't 10px the best practice?)

you’d set font-size: 62.5%, then have 1rem map to 10px so the conversion is simpler, 2.4rem == 24px

Changing this outright in Atlas would break any theme currently extending it.

1) We could create a function that returns a rem value based on the document's base font size and a pixel value. We would have to wrap every rem value we use in that function. We would also have to overwrite all Bootstrap 4 variables that use rem values with this new pattern. I'm also not sure if this will work 100%. I've done a limited test and it's working but there are probably edge cases.

2) Another option is to provide a base 10px or 62.5% variable theme in the compatibility layer.

3) The final option I can think of is to offer a third party variable theme with base 10px or 62.5%.

All 3 of these options will only provide support for Sass inside our themes. Any Sass provided by our widgets and other plugins will have to be customized.

[sbaechler]

Clay already uses a Sass variable called $font-size-base. That variable should be set to the root font size. It is then possible to use this instead of 1rem without breaking the layout no matter what the base font size is.

Currently, there are still a few places in the styles where there is a hardcoded value of 1rem. This should be replaced with $font-size-base. A 0.8rem value can be replaced with $font-size-base * 0.8.

In https://github.com/liferay/liferay-themes-sdk/issues/44 I have noted some places that should be fixed.

[pat270]

Moving the discussion from https://github.com/liferay/clay/pull/1332#issuecomment-442259890 so it's more visible.

Hi Patrick Let me explain the idea behind this. In fact, it does make a difference. $font-size-base is the size of the body text. This can now be decoupled from the rem value.

If you want to define 1rem as 10px while keeping a base font size of 16px you can do

$font-size-base: 1.6rem; html { font-size: 10px; } This is what you actually have to do now if you upgraded form a Liferay 7 site to 7.1 since Liferay 7 had a 10px root font size and 7.1 has 16px.

I don't think Bootstrap will change this variable. The implications would be too big. But if you prefer, we could have a $clay-font-size-base variable to separate the Liferay styles from the public UI.

I would also change the line widths to Pixel values. Otherwise you get fractions which don't render nicely.

Ok this is making sense now. We have some constraints we need to consider before we make a change like this:

1. We have Clay Base that extends Bootstrap which has hardcoded rem values based on 16px. We're currently unable to modify Bootstrap's variables directly and will need to import any variable changes before Bootstrap's variables.
2. Liferay CE/DXP has hardcoded rem values based on 16px that we can't easily convert. We have widget CSS that doesn't have access to Sass variables in the theme and can't scale using $clay-font-size-base.
3. There are themes based on Clay CSS that will break if we introduce this by default.

I'm still leaning toward an opt-in variable theme with base 10px since we will need to be importing variables anyways and this will minimize any breakages. We can implement @sbaechler 's suggestion by setting $font-size-base: 1.6rem and propagating it to all the component variables. It will look something like:

_clay-base-theme-with-base-10px.scss

// Setting to enable changing the document's base font size
$enable-base-10: false !default;

@if ($enable-base-10) {
    // The font size on the html element
    $document-font-size: 10px !default;

    // This is the base font size used for all component variables
    $clay-font-size-base: 1.6rem !default; // 16px

    // This sets font size on body element
    $font-size-base: 1.6rem !default; // 16px

    $border-radius: ($clay-font-size-base * 0.25) !default; // 4px
    $border-radius-lg: ($clay-font-size-base * 0.375) !default; // 6px
    $border-radius-sm: ($clay-font-size-base * 0.1875) !default; // 3px

    $btn-padding-x: ($clay-font-size-base * 0.75) !default; // 12px
    $btn-padding-y: ($clay-font-size-base * 0.375) !default; // 6px

    $btn-padding-x-sm: ($clay-font-size-base * 0.5) !default; // 8px
    $btn-padding-y-sm: ($clay-font-size-base * 0.75) !default; // 12px

    $btn-padding-x-lg: $clay-font-size-base !default; // 20px
    $btn-padding-y-lg: ($clay-font-size-base * 0.625) !default; // 10px
}

This seems a bit overboard honestly, but I can see a use case where someone picked an arbitrary font size value for the document and built their theme around it. If we decide to go forward with this, it would only apply partially to theme CSS in CE/DXP.

The CSS for the Product Menu and Control Menu are included in the document as a separate stylesheet and it doesn't have access to any of the Clay CSS Sass variables. This idea also applies to widget specific CSS. It would be up to you guys to overwrite any extraneous theme / module specific CSS yourself.

This could be more or less work depending on your use case. There is also a site that is a WIP https://v2-claypaver.wedeploy.io/. We are planning to add a px to rem converter that could make it easier to convert values no matter the document base font size.

[sbaechler]

About those constraints, I just checked the Bootstrap source for hardcoded rem variables and I only found 2 locations: One is the margin above the close button in the alert widget and another one is in the forms padding. The forms are highly customised by Liferay anyways so this should not be an issue.

I have been using Bootstrap 4 based themes with 10px base font size without any problems for years now.

Current themes should not break if $font-size-base is set to 1rem. However, they are already breaking now because the base font size has changed from 10px to 16px with the 7.1 update.

I don't really know what to say about the hardcoded widgets that cannot be overriden. I assume they are in the same Bootstrap 4 namespace as the rest of the site. This basically forces everyone to rewrite their themes completely when they upgrade from Liferay 7.0 to 7.1.

There is also the bug that the form plugin overrides the custom form styles with the default ones.

I am not convinced about the benefits of the opt-in. The title is currently misleading. Even though it is called base-10 the base value can be overridden to any value. Also I think $clay-font-size-base should have $font-size-base as default. But having two different values for clay base and the default base would break the consistency of the whole UI since clay is very invasive.

[pat270]

I just checked the Bootstrap source for hardcoded rem variables and I only found 2 locations

I'm probably misunderstanding what you mean by hardcoded rem variables. I was assuming any variable with the value {number}rem. A quick search of https://github.com/twbs/bootstrap/blob/8a628b943cf31ca0a002c08af661a95772480225/scss/_variables.scss shows me 94 places.

I have been using Bootstrap 4 based themes with 10px base font size without any problems for years now.

You sure about this? https://github.com/twbs/bootstrap/releases/tag/v4.0.0. Bootstrap 3 was base 10px. Bootstrap 4 is browser default which is generally 16px.

Current themes should not break if $font-size-base is set to 1rem.

By current themes, I'm referring to brand new themes created for 7.1 and not themes from 6.2 or 7.0. We can't be 100% sure of not breaking new themes created for 7.1 since anything could have been done with the variables at this point.

6.2 (Bootstrap 2) and 7.0 (Bootstrap 3) themes are expected to break since Bootstrap 4 is a complete rewrite. We're just trying to minimize the breakages from Bootstrap 3 to Bootstrap 4 which is what the opt-in theme is trying to accomplish.

I don't really know what to say about the hardcoded widgets that cannot be overriden. I assume they are in the same Bootstrap 4 namespace as the rest of the site.

I was referring to Liferay Portal specific CSS (not in the Clay repo) that uses a mix of px and rem values. Applying base 10px will change the sizing of Liferay Portal specific components that use hardcoded rem values that were based on 16px. This is more of an issue for someone that decides to base their 7.1 theme on 10px values.

I am not convinced about the benefits of the opt-in.

The benefit of opt-in essentially allows us to keep the code base the same so we don't break new 7.1 themes. The changes that you are suggesting are basically opt-in since you would need to change the documents font size to 10px and $font-size-base: 1.6rem. My suggestion in the previous comment makes the changes in a separate file so we don't run into unintended consequences for new 7.1 themes.

The title is currently misleading.

I named it quickly to help illustrate my thought process.

[jbalsas]

Hey @pat270, is this long standing PR somewhat related? -> RFS (Responsive font size) implementation

Could we build on top of it?

[pat270]

@jbalsas It looks like it would work. We would have to update our source to use the font-size mixin, but I can't say for sure. I'll grab that pull and test it out.

[pat270]

@jbalsas the RFS feature wasn't what I expected. It scales font-sizes based on viewport width at a specific max-width breakpoint. It doesn't provide any of the things we need for supporting base 10 font size. We can't apply this to things like padding, margins, height, width, etc. It also gives us odd font sizes like 15.2173 which wouldn't fly with the Lexicon team.

[marcoscv-work]

Hey guys, I'm super late with this but I would like to contribute with my point of view.

Since we selected bootstrap as a base for our own framework I think we had in mind how to make easier the use bootstrap base themes. Most of them are built by new components or default components variations with its own sizes over bootstrap.css (the compiled file). Some of them are built using the scss files changing some variables but adding its own sizes taking into account the bootstrap bases.

About components, I think it's happening some similar, in general people use bootstrap default font sizes as a reference, some examples here: https://bootsnipp.com/tags/4.0.0

I know the proposed change is not the biggest change in the CSS world but I feel is a big base change and can make it more difficult to not-advance-users.

So in order to make ease to reuse themes and components without worries about general sizes, I would keep the default BS4 font size base.

Of course, we could add some help to make the conversion, like mixins to convert in the 10px/1rem ratio. or any other help that we think can be useful.

[marcoscv-work]

Hey guys,

I've sent this PR as an example: https://github.com/liferay/clay/pull/1435

We and 'Bootstrap' are comfortable and in agreement with the rem units and the current state, but we can understand the problem they can represent for some users, so we could offer a mixin to use px that will be converted to rem by default.

[marcoscv-work]

@pat270 @sbaechler thoughts?

[sbaechler]

@marcoscv-work Please don't pivot this issue. The problem is the fixed base font size. It has nothing to do with the units.

Your MR still assumes a base font size of 16 Pixels and therefore fixes nothing.

The size of 1rem is defined as the font size of the html (root) element. In Liferay 7 it was 10px, in Liferay 7.1 it is 16px.

[jbalsas]

Hey @sbaechler, thank you for your patience with this issue.

We've been discussing this and trying to find a solution that would make us comfortable and still serve your use case the best way we can. Be sure that neither @pat270 nor @marcoscv-work are trying to pivot the issue but to find the best approach for all of us. We think making this change in Clay and breaking away from the Bootstrap convention will bring more problems down the road, so we want to stay as close to it as possible.

One option we're considering and @pat270 suggested would be to create a PostCSS plugin that we could ship by default with the theme tooling and that would convert the units on build time.

Do you think that could address your issue in a satisfactory way?

[sbaechler]

Hey @jbalsas

Are you suggesting to add a postcss plugin, that multiplies all rem values with a constant factor of $html-font-size/16 ?

This would add a discrepancy between the sass code and what is actually sent to the browser. It would be really confusing to work with the Developer tools because the values shown are not the values that are in the source code.

[jbalsas]

Hey @sbaechler, yeah, something like that. I think that discrepancy should be covered by providing the proper sourcemap to point back to the original code.

[sbaechler]

In that case I would rather provide a script that does a one-time transformation of the codebase.

But then you would be forcing everyone to use a root font size of 16px.

This would solve the problem of upgrading a theme from Liferay 7 to 7.1.

[jbalsas]

That's the other option, we could do that when upgrading the theme. We could prompt to ask if you wanted to do that one-time transformation.

[kresimir-coko]

Closing this due to inactivity, @sbaechler feel free to reopen with an update on your request.