fix(core): keep sign of negative numbers in convertToAbbreviationString - #2850
Conversation
🦋 Changeset detectedLatest commit: b3af10e The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for ix-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: siemens/ix/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthrough
ChangesAbbreviation Sign Handling
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~8 minutes Change: Bug fix · Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to Negative abbreviated values retain their sign, with tests for thousand and million values. No actionable merge-blocking risk is evident; the PR appears ready after normal checks. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
Looks good thank you 🎉 |



What is the current behavior?
Root cause:
convertToAbbreviationString(exported from@siemens/ix) comparesMath.abs(num)against each unit and then overwritesnumwith the rounded absolute value, so the minus sign is lost.convertToAbbreviationString(-1500)returns1.5KandconvertToAbbreviationString(-2000000)returns2M. Numbers with an absolute value below 1000 are not affected because they never enter that branch.GitHub Issue Number: none
What is the new behavior?
Math.sign(num), so-1500returns-1.5Kand-2000000returns-2M. Positive numbers are unchanged, and rounding stays symmetric (-1450gives-1.5Klike1450gives1.5K).utils/test/rwd.util.spec.ts. The negative case fails before the fix (expected '1.5K' to be '-1.5K') and passes after it.@siemens/ix.Checklist
A pull request can only be merged if all of these conditions are met (where applicable):
pnpm test)pnpm lint)pnpm build, changes pushed)Help & support
Validation: the new spec passes with
vitest run --project spec, ESLint and Prettier pass on the changed files, andstencil buildsucceeds for@siemens/ix. No visual change, so no visual regression or docs updates are needed.Summary by CodeRabbit
-1.5Kand-2M.