fix(typography): Use unquote for setting font-family in variables. #4665
Conversation
|
All 639 screenshot tests passed for commit d04972a vs. |
|
All 639 screenshot tests passed for commit 81d1869 vs. |
| @@ -22,7 +22,7 @@ | |||
|
|
|||
| @import "./functions"; | |||
|
|
|||
| $mdc-typography-font-family: Roboto, sans-serif !default; | |||
There was a problem hiding this comment.
Does our override mixins break if we set without quotes?
There was a problem hiding this comment.
Which override mixins? The mdc-typography mixins ?
There was a problem hiding this comment.
Yes, mixins / functions that use this variable.
There was a problem hiding this comment.
This variable is used below in the font styles map, but I would assume that our screenshot tests would break if the mixins/functions would break with this change.
There was a problem hiding this comment.
unquote("Roboto, sans-serif") seems to be same as Roboto, sans-serif. I looked at our typography mixins / functions, we don't seem to validate this type anywhere.
$fonts: unquote("Arial, sans-serif");
@debug type-of($fonts); // Return string
$fonts: Arial, sans-serif;
@debug type-of($fonts); // Return list
font-family: $fonts; // font-family can accept both list & string type.Having said that this change looks good to me. But sass automatically type casts the list to string when applied to CSS property as mentioned above.
WDYT?
There was a problem hiding this comment.
I think either way works for me, as long as we're consistent in both what we recommend users use (via docs) and what we use.
|
@williamernest did you want to merge this? |
|
Yes. |
|
All 687 screenshot tests passed for commit e50f494 vs. |
williamernest
deleted the
fix/typography/update-font-family-to-use-unquote
branch
While this did technically work, it semantically doesn't match the way we recommend users set the font-family.