fix(ui): reduce badge width overestimation in fallback text measurement#2199
fix(ui): reduce badge width overestimation in fallback text measurement#2199MathurAditya724 wants to merge 5 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaces fixed per-character width constants with an estimateTextWidth(text, fallbackFont) heuristic that classifies characters (narrow/medium/digit/uppercase/other) and applies font-specific coefficients for Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 Tip CodeRabbit can use oxc to improve the quality of JavaScript and TypeScript code reviews.Add a configuration file to your project to customize how CodeRabbit runs oxc. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/api/registry/badge/[type]/[...pkg].get.ts (1)
75-103: Externalise width coefficients and add a brief calibration note.The embedded literals (
3,5,6,7,5.5) make future tuning brittle. Moving them into named constants (per font profile) will make this safer to adjust.♻️ Proposed refactor
+const FALLBACK_WIDTHS = { + default: { + narrow: 3, + medium: 5, + digit: 6, + uppercase: 7, + other: 6, + }, + shieldsio: { + narrow: 3, + medium: 5, + digit: 6, + uppercase: 7, + other: 5.5, + }, +} as const + function estimateTextWidth(text: string, fallbackFont: 'default' | 'shieldsio'): number { + // Heuristic coefficients tuned to keep fallback rendering close to canvas metrics. + const widths = FALLBACK_WIDTHS[fallbackFont] let totalWidth = 0 for (const character of text) { if (NARROW_CHARS.has(character)) { - totalWidth += 3 + totalWidth += widths.narrow continue } if (MEDIUM_CHARS.has(character)) { - totalWidth += 5 + totalWidth += widths.medium continue } if (/\d/.test(character)) { - totalWidth += 6 + totalWidth += widths.digit continue } if (/[A-Z]/.test(character)) { - totalWidth += 7 + totalWidth += widths.uppercase continue } - totalWidth += fallbackFont === 'shieldsio' ? 5.5 : 6 + totalWidth += widths.other } return Math.max(1, Math.round(totalWidth)) }As per coding guidelines "Add comments only to explain complex logic or non-obvious implementations".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b026675c-7f1b-4eb9-ade0-52af6d8595bb
📒 Files selected for processing (1)
server/api/registry/badge/[type]/[...pkg].get.ts
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/api/registry/badge/[type]/[...pkg].get.ts (1)
90-98: Consider pre-compiling regex patterns outside the loop.The regex literals
/\d/and/[A-Z]/are recreated on each character iteration. For short badge text this has negligible impact, but pre-compiling them as module-level constants would be marginally more efficient and consistent with the existingNARROW_CHARSandMEDIUM_CHARSsets.♻️ Optional: pre-compile regex patterns
const MEDIUM_CHARS = new Set([ // ... existing entries ]) +const DIGIT_PATTERN = /\d/ +const UPPERCASE_PATTERN = /[A-Z]/ function estimateTextWidth(text: string, fallbackFont: 'default' | 'shieldsio'): number { // ... - if (/\d/.test(character)) { + if (DIGIT_PATTERN.test(character)) { totalWidth += 6 continue } - if (/[A-Z]/.test(character)) { + if (UPPERCASE_PATTERN.test(character)) { totalWidth += 7 continue } // ... }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3407ba18-2f99-4d5b-9aa3-a9316d542e95
📒 Files selected for processing (1)
server/api/registry/badge/[type]/[...pkg].get.ts
1 participant
🔗 Linked issue
Closes #2187
🧭 Context
Badge widths looked too wide in production for long values (for example https://main.npmx.dev/api/registry/badge/engines/vitest), while local dev looked tighter.
The root cause is that when exact canvas measurement is unavailable, the fallback used a fixed per-character width, which overestimates strings containing many narrow characters (spaces, separators, punctuation).
📚 Description
This updates the badge text-width fallback logic to be less aggressive and more realistic, replace the fixed-width fallback with a small character-class estimator