fix: handle font-stretch returned by font providers#345
fix: handle font-stretch returned by font providers#345florian-lefebvre wants to merge 4 commits intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdded parsing support for CSS Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/css/parse.ts`:
- Around line 134-136: The Percentage branch is pushing percents directly into
values causing arrays like ["50%","200%"] instead of a space-joined string;
update the Percentage handling in the same function (the child.type ===
'Percentage' case) to append the formatted percent (e.g. `${child.value}%`) into
the existing buffer (the same pattern used for Dimension handling) rather than
calling values.push, so that subsequent buffer flushing produces a single
space-separated string (matching the stretch?: string type in src/types.ts).
In `@test/extract.test.ts`:
- Around line 208-230: The test uses a quoted percentage which css-tree parses
as a String, so the new Percentage handling path in parse.ts isn't exercised;
update the test in extract.test.ts to use an unquoted percentage (e.g.,
font-stretch: 110%) so that extractFontFaceData(...) triggers the Percentage
node branch in parse.ts (the code handling Percentage values) and the snapshot
reflects the parsed "110%" stretch value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 401aa789-cb90-4b10-ba9a-0a4787b86110
📒 Files selected for processing (3)
src/css/parse.tstest/extract.test.tstest/providers/adobe.test.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #345 +/- ##
=======================================
Coverage 98.33% 98.33%
=======================================
Files 12 12
Lines 660 662 +2
Branches 172 173 +1
=======================================
+ Hits 649 651 +2
Misses 11 11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
It was reported on Astro's discord that the following would not have impact on the font stretch when it should:
Turns out
font-stretchwasn't coveredSummary by CodeRabbit
New Features
Tests