patternfly icon indicating copy to clipboard operation
patternfly copied to clipboard

fix(global): added custom property to override root font-size

Open mcoker opened this issue 3 years ago • 2 comments

fixes https://github.com/patternfly/patternfly/issues/5028

This PR allows you to set a custom font-size with this rule that was added to PF4 to override PF3's font-size on html

https://github.com/patternfly/patternfly/blob/9ab78a81b9d6d63cd7795fa82b1ff673abba5332/src/patternfly/base/_common.scss#L12-L21

An issue with updating this rule is that it's so fundamental, if we introduce a change that inadvertently causes an issue with the way someone is using PF now, it would likely impact the entire site/app.

The solution in this PR is probably the simplest approach, though it does require a value passed for the font-size, which may be a limitation since you can't allow html to inherit its font-size from some other dynamic/unknown style. To override the font-size with this PR, you would just declare the custom property like so:

:root {
  --pf-global--root--FontSize: [custom value];
}

Would this work @Marc19 and @warmbowski? I like this because it doesn't add any bloat, and is overridable like any of our other global vars.

Other options

  1. Update the current selector that unsets the font-size currently to:

    html:where(:not(.pf-no-root-font-size)) {
      font-size: var(--pf-global--root--FontSize, unset) !important;
    }
    
    • This would allow an opt-out at the class level to disable the declaration entirely and inherit from some other system without having to explicitly set the font-size. This seems like the best of both worlds - you can opt out of the declaration entirely with a class on the html element, and override the size with a custom value if preferred.
      • My only hesitation with this is that :where() was introduced relatively recently (though it's within our officially supported browsers) and may break some users, which we always try to avoid if we can unless we're fixing a critical bug and there is no other workaround. We could write a fallback with @supports not selector(:where(...)) {} with the original rule, but @supports selector support is similar to :where(), with safari at least.
  2. Add another compiled stylesheet that omits the rule by setting $pf-global--unset-root-font-size: false;, similar to patternfly-base-no-reset.scss. This would also be fine, but I'd like to avoid adding another compiled stylesheet if possible since it has the potential to get a little complicated to combine different features in these custom compiled variations.

cc @srambach @mattnolting

mcoker avatar Aug 11 '22 19:08 mcoker

Preview: https://patternfly-pr-5032.surge.sh

A11y report: https://patternfly-pr-5032-a11y.surge.sh

patternfly-build avatar Aug 11 '22 19:08 patternfly-build

Hey @mcoker, I think this should work for us 👍 Thanks for addressing this!

Marc19 avatar Aug 16 '22 07:08 Marc19

@warmbowski - just wanted to check and see if this solution will work for you. FYI we have code freeze EOD today, so if we can get it merged it will be in our next release.

mcoker avatar Aug 17 '22 14:08 mcoker

This works for me. Thanks!

warmbowski avatar Aug 17 '22 21:08 warmbowski

:tada: This PR is included in version 4.208.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

patternfly-build avatar Aug 17 '22 22:08 patternfly-build