fix(fonts): unquote unicode-range and font-display values #19511
Conversation
petersandor
force-pushed
the
firefox-quoted-style-props
branch
from
e3a7fb0 to
1f7704c
Compare
✅ Deploy Preview for v11-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for v11-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #19511 +/- ##
=======================================
Coverage 84.48% 84.48%
=======================================
Files 373 373
Lines 14641 14641
Branches 4839 4843 +4
=======================================
Hits 12369 12369
Misses 2124 2124
Partials 148 148 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Still seeing some warnings, and not able to test to make sure what's fixed. Can you please make the PR description a bit more elaborate. |
@Gururajj77 The purpose of this fix is to remove quotes from |
| $items: map.get($ranges, $name); | ||
| $count: list.length($items); | ||
| $result: ''; | ||
|
|
||
| @for $i from 1 through $count { | ||
| $result: if($i == 1, '', $result + ', ') + list.nth($items, $i); | ||
| } | ||
|
|
||
| @return string.unquote($result); |
There was a problem hiding this comment.
Did you consider modifying the $ranges map above to contain unquoted strings instead of quoted strings? What you have here is fine, just curious if there's a tradeoff or blocker that pushed you in this direction. The sass docs mention CSS property values being a good time to use unquoted strings.
There was a problem hiding this comment.
@tay1orjones That was the first thing that I tried but it didn't seem to work, must've messed up on my side. I opened #19588 to revert this.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
348d58f
…ign-system#19511) * fix(fonts): unquote unicode-range and font-display values Fixes carbon-design-system#19298 Fixes carbon-design-system#19324 * chore: added myself as a contributor
Closes #19298
Closes #19324
Changelog
Changed
unicode-rangeandfont-displayvalues are now without quotesTesting / Reviewing
unicode-rangeandfont-displayPR Checklist
As the author of this PR, before marking ready for review, confirm you:
[ ] Updated documentation and storybook examples[ ] Wrote passing tests that cover this change[ ] Addressed any impact on accessibility (a11y)More details can be found in the pull request guide