fix(echarts): Apply D3 locale formatting to adaptive datetime x-axis#34661
fix(echarts): Apply D3 locale formatting to adaptive datetime x-axis#34661
Conversation
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
I've completed my review and didn't find any issues.
Files scanned
| File Path | Reviewed |
|---|---|
| superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts
Outdated
Show resolved
Hide resolved
rusackas
force-pushed
the
d3-localization
branch
from
32353ad to
15352d3
Compare
PR Updated - Rebased and Review Feedback AddressedChanges Made
Current Behavior
This ensures that when users select "Adaptive" datetime format, the D3 locale settings (like Russian month names) are properly applied. Summary of the FixThe root issue was that |
Code Review Agent Run #157a43Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
Retrying CI - previous failures were in unrelated cypress tests |
Fixes #31790 When the "Adaptive" datetime format option was selected for x-axis in ECharts visualizations, the custom D3 locale settings (like localized month names) were being ignored. This was because getXAxisFormatter() was returning undefined for SMART_DATE_ID, causing ECharts to use its default formatter instead of the configured D3 locale formatter. The fix ensures that when SMART_DATE_ID is selected, the smart date formatter with proper locale support is returned, allowing custom D3 time formats with localized month and day names to work correctly with the adaptive option. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Address review feedback from @michael-s-molina: Since the function no longer returns String, remove StringConstructor from the return type and update the misleading test description. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
rusackas
force-pushed
the
d3-localization
branch
from
15352d3 to
65cfebd
Compare
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sequence DiagramThe PR ensures the x-axis formatter returns the smart date (D3-aware) formatter when the "Adaptive" (SMART_DATE_ID) option is selected so localized month/day names are used. The diagram shows the main success path and the alternative paths for custom or absent formats. sequenceDiagram
participant Viz as Visualization
participant Util as XAxis Formatter Util
participant TimeFmt as TimeFormatter (D3 locale-aware)
participant ECharts
Viz->>Util: request x-axis formatter (format = SMART_DATE_ID)
Util->>TimeFmt: getSmartDateFormatter() (D3 locale)
TimeFmt-->>Util: TimeFormatter (locale-aware)
Util-->>ECharts: return TimeFormatter
ECharts-->>Viz: render x-axis labels (localized month/day names)
alt format is custom string
Viz->>Util: request x-axis formatter (format = "%Y-%m-%d")
Util->>TimeFmt: getTimeFormatter(format)
Util-->>ECharts: return TimeFormatter
else no format specified
Viz->>Util: request x-axis formatter (no format)
Util-->>ECharts: return undefined (ECharts default)
end
Generated by CodeAnt AI |
Code Review Agent Run #b835e5Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Update tests to reflect the intentional behavior change where smart_date now returns a formatter function instead of undefined. The first test now explicitly sets xAxisTimeFormat to undefined to test the no-format case, and the test.each loop now expects all formats including smart_date to return a formatter function. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #b339f3Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
2 participants
Summary
Fixes
Fixes #31790
Problem
When users configured custom D3 time formats with localized month and day names (e.g., Russian), these localizations were ignored when the "Adaptive" datetime format option was selected for the x-axis in line charts and other ECharts visualizations. The formatting would revert to English month names instead of using the configured locale.
Solution
Modified
getXAxisFormatter()insuperset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.tsto return the smart date formatter (which respects D3 locale settings) whenSMART_DATE_IDis specified, instead of returningundefined.Testing Instructions
superset_config.pywith localized month/day namesTest Coverage
Added comprehensive unit tests for
getXAxisFormatter()to ensure:🤖 Generated with Claude Code