feat(i18n): format numbers with Intl.NumberFormat#1149
feat(i18n): format numbers with Intl.NumberFormat#1149danielroe merged 7 commits intonpmx-dev:mainfrom
Intl.NumberFormat#1149Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
Does it make sense to test browser APIs? 🤔 |
📝 WalkthroughWalkthroughThis pull request centralises numeric and byte formatting by adding Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/composables/useNumberFormatter.ts (1)
14-32: Align byte unit labels with the base used.The formatter uses 1024 multiples but labels values as kB/MB (SI). Consider switching to 1000-based units or renaming labels to KiB/MiB to avoid unit mismatch.
Possible adjustment (1000‑based)
- format: (bytes: number) => { - if (bytes < 1024) + format: (bytes: number) => { + const kB = 1000 + const MB = 1000 * 1000 + if (bytes < kB) return t('package.size.b', { size: decimalNumberFormatter.value.format(bytes), }) - if (bytes < 1024 * 1024) + if (bytes < MB) return t('package.size.kb', { - size: decimalNumberFormatter.value.format(bytes / 1024), + size: decimalNumberFormatter.value.format(bytes / kB), }) return t('package.size.mb', { - size: decimalNumberFormatter.value.format(bytes / (1024 * 1024)), + size: decimalNumberFormatter.value.format(bytes / MB), }) },app/composables/usePackageComparison.ts (1)
352-354: Consider locale-formatting dependency counts too.Now that formatters are injected, dependency and total dependency counts are still rendered via
String(), so they won’t be locale-formatted. Adding a plain number formatter here would make the facet values fully consistent with the PR goal.app/components/Package/Dependencies.vue (1)
153-199: Format the remaining “show all” counts for consistency.
Peer/optional sections still pass raw lengths, while other counts are now formatted.♻️ Suggested tweak
$t('package.peer_dependencies.show_all', { - count: sortedPeerDependencies.length, + count: numberFormatter.format(sortedPeerDependencies.length), }) $t('package.optional_dependencies.show_all', { - count: sortedOptionalDependencies.length, + count: numberFormatter.format(sortedOptionalDependencies.length), })Also applies to: 201-242
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/components/Package/Dependencies.vue (2)
203-241:⚠️ Potential issue | 🟡 MinorOptional dependencies “Show all” count is still unformatted.
Line 239 still passes the raw length; use the number formatter for locale consistency.Proposed fix
{{ $t('package.optional_dependencies.show_all', { - count: sortedOptionalDependencies.length, + count: numberFormatter.format(sortedOptionalDependencies.length), }) }}
139-143:⚠️ Potential issue | 🟡 MinorRemove per-element focus-visible utilities on buttons.
These buttons still applyfocus-visible:outline-accent/70; the project uses a global button/select focus-visible rule, so inline utilities should be dropped.Based on learnings, “In the npmx.dev project, ensure that focus-visible styling for button and select elements is implemented globally in app/assets/main.css with the rule: button:focus-visible, select:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; border-radius: 4px; }. Do not apply per-element inline utility classes like focus-visible:outline-accent/70 on these elements in Vue templates or components.”Proposed fix
- class="my-2 ms-1 font-mono text-xs text-fg-muted hover:text-fg transition-colors duration-200 rounded focus-visible:outline-accent/70" + class="my-2 ms-1 font-mono text-xs text-fg-muted hover:text-fg transition-colors duration-200 rounded"- class="mt-2 font-mono text-xs text-fg-muted hover:text-fg transition-colors duration-200 rounded focus-visible:outline-accent/70" + class="mt-2 font-mono text-xs text-fg-muted hover:text-fg transition-colors duration-200 rounded"Also applies to: 187-191
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
…/number-locale-formatting
|
@DDeenis is attempting to deploy a commit to the serhalp's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
It seems that the PR for removed from the merge queue for unrelated reasons. Anyway, I've implemented coderabbit's suggestion. |
2 participants
Format all numbers relative to the current locale. Let me know if I missed any.
Package versions are NOT locale formatted on purpose - they can be used for installing a specific version of the package, and not necessarily have only numbers.
Example (package page - Arabic)