feat: add Excel Charts #146
Conversation
|
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #146 +/- ##
=========================================
+ Coverage 100.0% 100.0% +0.1%
=========================================
Files 24 25 +1
Lines 1132 1342 +210
Branches 93 112 +19
=========================================
+ Hits 1131 1341 +210
Misses 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // --------------------------- | ||
| // Chart related interfaces | ||
| // --------------------------- | ||
| export type ChartType = 'bar' | 'line' | 'pie' | 'scatter'; |
There was a problem hiding this comment.
by looking at w3school most popular Excel Charts, we should rename bar to column (bottom-top) and then add bar as a left-right chart. Also might be a good idea to add stacked charts for both bar/column charts.
| series?: ChartSeriesRef[]; | ||
| /** Legacy single-series fallback: categories literal */ | ||
| categories?: string[]; | ||
| /** Legacy single-series fallback: values literal */ |
There was a problem hiding this comment.
why legacy? is that for legacy Excel file? I might need explanation and possibly (probably) just drop any legacy stuff. Would that simplify code to drop any legacy thing?
| /** Hex ARGB or RGB color (e.g. FF0000 or FF0000FF) - currently cosmetic placeholder */ | ||
| color?: string; | ||
| /** For scatter charts: X axis values range */ | ||
| xValuesRange?: string; |
There was a problem hiding this comment.
why do we have xValuesRange and not a yValuesRange? it seems that the valuesRange is the y and so why?
| /** Category axis title (ignored for pie) */ | ||
| xAxisTitle?: string; | ||
| /** Value axis title (ignored for pie) */ | ||
| yAxisTitle?: string; |
There was a problem hiding this comment.
instead of having 2 separate props, could we merge them and use something like interface AxisOptions { title?: string; minimum?: number; maximum?: number'; showGridLines?: boolean } axis: { x?: AxisOptions; y?: AxisOptions; }? This is probably better for extensibility since for a 3D, then we would also have a z title.
we should do something similar for legend
see https://learn.microsoft.com/en-us/office/dev/add-ins/excel/excel-add-ins-charts
| /** Height in EMUs */ | ||
| height?: number; | ||
| /** Worksheet name containing referenced ranges */ | ||
| sheetName?: string; |
There was a problem hiding this comment.
is this sheetName necessary? what does that do for Charts?
1 participant
Uh oh!
There was an error while loading. Please reload this page.