feat(component): add number formatting to single value and tooltips - #169
Closed
alfredo1996 wants to merge 3 commits into
Closed
feat(component): add number formatting to single value and tooltips#169alfredo1996 wants to merge 3 commits into
alfredo1996 wants to merge 3 commits into
Conversation
Add shared formatNumber() and buildTooltipFormatter() to chart-utils.ts: - Decimal places config (0-6, -1 for automatic) - Comma/compact/percent formatting - Prefix/suffix support - Consistent tooltip formatting across all ECharts chart types Add decimalPlaces option to single-value, bar, line, and pie chart schemas. Update SingleValueChart to use the shared formatter. Closes #138 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
alfredo1996
added
enhancement
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughIntroduced number formatting utilities ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
component/src/charts/chart-utils.ts (1)
32-41: Hardcodeden-USlocale for number formatting.The locale is hardcoded to
"en-US"for comma formatting and"en"for compact notation. This works for the current use case but may cause issues for international users expecting their locale's conventions.Consider parameterizing locale in
NumberFormatConfigif i18n is a future requirement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@component/src/charts/chart-utils.ts` around lines 32 - 41, The formatting branches for "comma" and "compact" are using hardcoded locales ("en-US" and "en"); update NumberFormatConfig to accept an optional locale string (e.g., locale?: string) and use that value when constructing toLocaleString/Intl.NumberFormat, falling back to a sensible default (like navigator.language or "en-US") when locale is undefined; adjust the "comma" case and the "compact" case to reference the new config.locale instead of the hardcoded strings so formatting respects the caller's locale preference.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@component/src/charts/chart-utils.ts`:
- Around line 79-83: The current items.map callback builds a label using
`${p.marker ?? ""} ${p.seriesName ?? ""}: <b>${val}</b>` which can produce "
undefined: <b>…</b>" or an extra colon/space when seriesName is missing; update
the template in the items.map (the arrow callback that assigns lines) to only
include the colon and seriesName when p.seriesName is present (e.g.
conditionally prepend `${p.seriesName}: ` or omit it), and ensure spacing around
p.marker (p.marker) is trimmed or added only when needed so the final string
never shows "undefined" or stray punctuation while still using formatNumber(val,
tooltipConfig) for value formatting.
In `@component/src/components/composed/chart-options-schema.ts`:
- Around line 373-375: The tooltip decimalPlaces option is currently inert
because tooltipFormatOptions (which includes decimalPlaces) is spread into the
chart option registries but the chart components (bar-chart.tsx, line-chart.tsx,
pie-chart.tsx) use hardcoded tooltip configs; either remove tooltipFormatOptions
from the bar/line/pie arrays or wire the formatter: call
buildTooltipFormatter(...) inside each component and replace the hardcoded
tooltip config with one that uses the returned formatter (ensure you
import/accept decimalPlaces from props and pass it to buildTooltipFormatter in
bar-chart.tsx, line-chart.tsx, and pie-chart.tsx so decimalPlaces controls
tooltip output).
---
Nitpick comments:
In `@component/src/charts/chart-utils.ts`:
- Around line 32-41: The formatting branches for "comma" and "compact" are using
hardcoded locales ("en-US" and "en"); update NumberFormatConfig to accept an
optional locale string (e.g., locale?: string) and use that value when
constructing toLocaleString/Intl.NumberFormat, falling back to a sensible
default (like navigator.language or "en-US") when locale is undefined; adjust
the "comma" case and the "compact" case to reference the new config.locale
instead of the hardcoded strings so formatting respects the caller's locale
preference.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 507d458d-743c-4a0e-8b75-f5e184ba13c8
📒 Files selected for processing (5)
component/src/charts/__tests__/format-number.test.tscomponent/src/charts/__tests__/single-value-chart.test.tsxcomponent/src/charts/chart-utils.tscomponent/src/charts/single-value-chart.tsxcomponent/src/components/composed/chart-options-schema.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ions - Fix TypeScript compilation error (unused NumberFormatConfig import) - Conditionally include seriesName in tooltip to avoid "undefined:" display - Remove inert decimalPlaces option from bar/line/pie chart schemas (only used in single-value where it's actually wired up) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
alfredo1996
pushed a commit
that referenced
this pull request
Mar 24, 2026
…els, markLine, pie donut) Merges PRs #169, #170, #171, #173, #174 into a single release branch. Resolves merge conflicts in chart-utils.ts, bar-chart.tsx, line-chart.tsx, and chart-options-schema.ts. Includes: - Number formatting for single-value and tooltips (#169) - DataZoom support for bar and line charts (#170) - Auto-rotate and truncate axis labels (#171) - Reference lines (markLine) for bar and line charts (#173) - Donut center text and Top-N grouping for pie chart (#174) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 tasks
Owner
Author
|
Superseded by consolidated PR #185 (release/chart-improvements) |
alfredo1996
pushed a commit
that referenced
this pull request
May 10, 2026
…els, markLine, pie donut) Merges PRs #169, #170, #171, #173, #174 into a single release branch. Resolves merge conflicts in chart-utils.ts, bar-chart.tsx, line-chart.tsx, and chart-options-schema.ts. Includes: - Number formatting for single-value and tooltips (#169) - DataZoom support for bar and line charts (#170) - Auto-rotate and truncate axis labels (#171) - Reference lines (markLine) for bar and line charts (#173) - Donut center text and Top-N grouping for pie chart (#174) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add shared number formatting utilities (
formatNumber,buildTooltipFormatter) tochart-utils.tsand wire them into the single-value chart. This creates the foundation for consistent tooltip formatting across all ECharts chart types.Changes
formatNumber()— decimal places (0-6), comma/compact/percent, prefix/suffixbuildTooltipFormatter()— ECharts tooltip formatter factory usingformatNumberSingleValueChart— uses shared formatter, newdecimalPlacespropchart-options-schema.ts—decimalPlacesoption added to single-value, bar, line, pieTest plan
cd component && npm test— 68 suites, 1031 tests passingcd app && npm test— 72 suites, 1260 tests passingnpm run build— no type errorsnpm run lint— cleanCloses #138
🤖 Generated with Claude Code
Summary by CodeRabbit
decimalPlacesconfiguration option for bar, line, pie, and single value charts to control decimal precision in displayed values.