-
Notifications
You must be signed in to change notification settings - Fork 18
Format VS Dashboard numeric values (CRASM-3590) #1424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
cduhn17
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - no major findings
jsalinasnttdata
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
a4439ea
The merge-base changed after approval.
cduhn17
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
🗣 Description
Standardizes large numeric values displays across the VS Dashboard. (CRASM-3590)
💭 Motivation and context
Any numeric values in the VS Dashboard that are larger than 3 digits will now have appropriate commas with locale-aware formatting (e.g. 3071 → 3,071) for easy user display while also preserving their numeric type for calculations and comparisons elsewhere in the application.
As part of this work, existing components were lightly refactored to centralize display logic and improve accessibility, and unit tests were expanded to validate both formatting behavior and edge cases.
Unit test files added:
🧪 Testing
📷 Screenshots (if appropriate)
Showing large numeric values now have commas with example test values
✅ Pre-approval checklist
bump_versionscript if this repository is versioned and the changes in this PR warrant a version bump.✅ Pre-merge checklist
✅ Post-merge checklist