-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set font-sizes relative to font-size-base #24060
Conversation
😄 nice, I like this one! What about |
scss/_variables.scss
Outdated
$h4-font-size: 1.5rem !default; | ||
$h5-font-size: 1.25rem !default; | ||
$h6-font-size: 1rem !default; | ||
$headings-font-size-base: $font-size-base !default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't introduce a new base for headers
@mdo @XhmikosR I took another look at this PR. We have several font-size references across _variables.scss, and since they are all set in rem (or %) it might be easier to just to change the font-size base definition from to under That way by changing the This would be a breaking change, and more dramatic than just making fonts sizes follow I am ok with either, if you think we should continue with it. I can send a PR PR or attach a commit to this one to clean it up depending on the direction we want to go. |
PR for joomla#31184 I took a look at this to create a PR the correct way for bootstrap scss We should just have to change one variable `$font-size-base: 0.875rem; //instead of 1rem` But the rest of the font variables have not been defined correctly (probably based on bs4 beta1?) and they should be changed as well to be relative to this variable. See the commit to bootstrap 3 years ago where this was done twbs/bootstrap#24060 There may be other font sizes in our scss that need to be updated to use this variable - I have not checked yet I have no opinion on a base font size of 16px (before) and 14px (with this PR) To test you will need to run `npm ci` or `node build.js --compile-css`
PR for joomla#31184 I took a look at this to create a PR the correct way for bootstrap scss We should just have to change one variable `$font-size-base: 0.875rem; //instead of 1rem` But the rest of the font variables have not been defined correctly (probably based on bs4 beta1?) and they should be changed as well to be relative to this variable. See the commit to bootstrap 3 years ago where this was done twbs/bootstrap#24060 There may be other font sizes in our scss that need to be updated to use this variable - I have not checked yet I have no opinion on a base font size of 16px (before) and 14px (with this PR) To test you will need to run `npm ci` or `node build.js --compile-css`
This PR closes #24056