-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fluid typography: add font size constraints #44993
Merged
ramonjd
merged 12 commits into
trunk
from
update/fluid-typography-with-min-font-size-value-constraint
Oct 18, 2022
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
db8dd97
Initial commit:
ramonjd 7027925
Rounding parsed min and max values
ramonjd 7e29b5f
Updating JS rounding function so that values are rounded
ramonjd 08b8903
Removing type coercion
ramonjd 087f78a
Updating failing test
ramonjd c8d80b4
Remove reference to preset in favour of "fontSize"
ramonjd f45b4a5
Ensure that rem units are used when calculating min and max boundarie…
ramonjd 9222dce
Oh Linter! You persistent crank.
ramonjd df6c71b
For font sizes of < 14px that have no defined minimum sizes, use the …
ramonjd aa2ddf7
Removing max < min check. It wasn't there before. It can be a follow …
ramonjd b65a8b6
Checking for a zero-based linear factor. If we find one, default to `1`
ramonjd b7a01f8
Refer to correct JS var in JS file
ramonjd File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
In the (unchanged) lines above, we're coercing the max size to
$font_size_unit
but not the min size, is that deliberate?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.
It is, and here's the logic (mine at least 😄)
The min and max font size units can theoretically differ, so one can be
px
and the otherrem
. This is fine inthe resulting clamp value's first and third args, e.g.,
clamp(20px, 1.25rem + ((1vw - 7.68px) * 93.75), 50rem)
But in order to calculate a linear factor between viewport boundaries we have to do the math on similar values, that is, values whose units are the same. So we coerce the max value unit to whatever the min value unit is.
I use the min value because, if there isn't a min value specified, e.g., it's a custom font size, we use incoming custom font size to calculate a minimum value, and therefore use its unit.
The functions' internals are ripe for refactor by the way, but since it's all neatly housed I think we can do it in a follow up.