feat: enhance chart tools with common styles and color palettes; add …#43
feat: enhance chart tools with common styles and color palettes; add …#43xwang152-jack wants to merge 1 commit into
Conversation
…CLAUDE.md for project guidance
|
@xwang152-jack 是修改图表的默认风格样式吗? |
There was a problem hiding this comment.
Pull request overview
Adds a centralized theming utility for ECharts (common styles + palettes) and updates multiple chart tools to use it, alongside repository guidance and gitignore tweaks.
Changes:
- Introduce
src/utils/theme.tswith shared palettes/styles and helper functions (apply styles, gradients, animations). - Update several chart tools (bar/line/pie/radar/scatter/sankey/heatmap/gauge/funnel) to apply the new theme helpers.
- Add
CLAUDE.mdguidance and ignore snapshot test artifacts in.gitignore.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/theme.ts | New shared palettes/common styling helpers for ECharts options |
| src/utils/index.ts | Re-export theme helpers from utils barrel |
| src/tools/bar.ts | Apply palette/common styles + new bar aesthetics |
| src/tools/line.ts | Apply common styles + animation config |
| src/tools/pie.ts | Apply palette/common styles + updated pie aesthetics |
| src/tools/radar.ts | Apply palette/common styles + updated radar aesthetics |
| src/tools/scatter.ts | Apply palette/common styles + updated scatter aesthetics |
| src/tools/sankey.ts | Apply palette/common styles + updated sankey aesthetics |
| src/tools/heatmap.ts | Apply common styles + updated visualMap palette and tooltip styling |
| src/tools/gauge.ts | Apply palette/common styles + updated gauge aesthetics |
| src/tools/funnel.ts | Apply palette/common styles + updated funnel aesthetics |
| CLAUDE.md | Add project/development guidance document |
| .gitignore | Ignore snapshot “actual/diff” artifacts from image tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ...COMMON_STYLES.axisLine, | ||
| ...COMMON_STYLES.splitLine, | ||
| ...COMMON_STYLES.axisLabel, | ||
| ...COMMON_STYLES.axisName, | ||
| ...axis, | ||
| })) | ||
| : { | ||
| ...COMMON_STYLES.axisLine, | ||
| ...COMMON_STYLES.splitLine, | ||
| ...COMMON_STYLES.axisLabel, | ||
| ...COMMON_STYLES.axisName, | ||
| ...option.xAxis, | ||
| } | ||
| : undefined, | ||
| yAxis: option.yAxis | ||
| ? Array.isArray(option.yAxis) | ||
| ? option.yAxis.map((axis) => ({ | ||
| ...COMMON_STYLES.axisLine, | ||
| ...COMMON_STYLES.splitLine, | ||
| ...COMMON_STYLES.axisLabel, | ||
| ...COMMON_STYLES.axisName, | ||
| ...axis, | ||
| })) | ||
| : { | ||
| ...COMMON_STYLES.axisLine, | ||
| ...COMMON_STYLES.splitLine, | ||
| ...COMMON_STYLES.axisLabel, | ||
| ...COMMON_STYLES.axisName, | ||
| ...option.yAxis, | ||
| } |
There was a problem hiding this comment.
applyCommonStyles is spreading COMMON_STYLES.axisLine/splitLine/axisLabel/axisName directly into the axis object. Since those constants currently contain the inner option objects (e.g. { lineStyle: ... }) rather than being nested under axisLine, splitLine, axisLabel, etc., this produces invalid ECharts axis configs (e.g. xAxis.lineStyle instead of xAxis.axisLine.lineStyle), so the styles won’t apply. Restructure the merge to set axisLine: COMMON_STYLES.axisLine, splitLine: COMMON_STYLES.splitLine, axisLabel: COMMON_STYLES.axisLabel, and use nameTextStyle (not axisName) for axis name styling, then merge user-provided axis options on top.
| ...COMMON_STYLES.axisLine, | |
| ...COMMON_STYLES.splitLine, | |
| ...COMMON_STYLES.axisLabel, | |
| ...COMMON_STYLES.axisName, | |
| ...axis, | |
| })) | |
| : { | |
| ...COMMON_STYLES.axisLine, | |
| ...COMMON_STYLES.splitLine, | |
| ...COMMON_STYLES.axisLabel, | |
| ...COMMON_STYLES.axisName, | |
| ...option.xAxis, | |
| } | |
| : undefined, | |
| yAxis: option.yAxis | |
| ? Array.isArray(option.yAxis) | |
| ? option.yAxis.map((axis) => ({ | |
| ...COMMON_STYLES.axisLine, | |
| ...COMMON_STYLES.splitLine, | |
| ...COMMON_STYLES.axisLabel, | |
| ...COMMON_STYLES.axisName, | |
| ...axis, | |
| })) | |
| : { | |
| ...COMMON_STYLES.axisLine, | |
| ...COMMON_STYLES.splitLine, | |
| ...COMMON_STYLES.axisLabel, | |
| ...COMMON_STYLES.axisName, | |
| ...option.yAxis, | |
| } | |
| axisLine: { | |
| ...COMMON_STYLES.axisLine, | |
| ...(axis && typeof axis === "object" && "axisLine" in axis | |
| ? (axis as any).axisLine | |
| : {}), | |
| }, | |
| splitLine: { | |
| ...COMMON_STYLES.splitLine, | |
| ...(axis && typeof axis === "object" && "splitLine" in axis | |
| ? (axis as any).splitLine | |
| : {}), | |
| }, | |
| axisLabel: { | |
| ...COMMON_STYLES.axisLabel, | |
| ...(axis && typeof axis === "object" && "axisLabel" in axis | |
| ? (axis as any).axisLabel | |
| : {}), | |
| }, | |
| nameTextStyle: { | |
| ...COMMON_STYLES.axisName, | |
| ...(axis && typeof axis === "object" && "nameTextStyle" in axis | |
| ? (axis as any).nameTextStyle | |
| : {}), | |
| }, | |
| ...axis, | |
| })) | |
| : option.xAxis | |
| ? { | |
| axisLine: { | |
| ...COMMON_STYLES.axisLine, | |
| ...(option.xAxis && typeof option.xAxis === "object" && "axisLine" in option.xAxis | |
| ? (option.xAxis as any).axisLine | |
| : {}), | |
| }, | |
| splitLine: { | |
| ...COMMON_STYLES.splitLine, | |
| ...(option.xAxis && typeof option.xAxis === "object" && "splitLine" in option.xAxis | |
| ? (option.xAxis as any).splitLine | |
| : {}), | |
| }, | |
| axisLabel: { | |
| ...COMMON_STYLES.axisLabel, | |
| ...(option.xAxis && typeof option.xAxis === "object" && "axisLabel" in option.xAxis | |
| ? (option.xAxis as any).axisLabel | |
| : {}), | |
| }, | |
| nameTextStyle: { | |
| ...COMMON_STYLES.axisName, | |
| ...(option.xAxis && typeof option.xAxis === "object" && "nameTextStyle" in option.xAxis | |
| ? (option.xAxis as any).nameTextStyle | |
| : {}), | |
| }, | |
| ...option.xAxis, | |
| } | |
| : undefined | |
| : undefined, | |
| yAxis: option.yAxis | |
| ? Array.isArray(option.yAxis) | |
| ? option.yAxis.map((axis) => ({ | |
| axisLine: { | |
| ...COMMON_STYLES.axisLine, | |
| ...(axis && typeof axis === "object" && "axisLine" in axis | |
| ? (axis as any).axisLine | |
| : {}), | |
| }, | |
| splitLine: { | |
| ...COMMON_STYLES.splitLine, | |
| ...(axis && typeof axis === "object" && "splitLine" in axis | |
| ? (axis as any).splitLine | |
| : {}), | |
| }, | |
| axisLabel: { | |
| ...COMMON_STYLES.axisLabel, | |
| ...(axis && typeof axis === "object" && "axisLabel" in axis | |
| ? (axis as any).axisLabel | |
| : {}), | |
| }, | |
| nameTextStyle: { | |
| ...COMMON_STYLES.axisName, | |
| ...(axis && typeof axis === "object" && "nameTextStyle" in axis | |
| ? (axis as any).nameTextStyle | |
| : {}), | |
| }, | |
| ...axis, | |
| })) | |
| : option.yAxis | |
| ? { | |
| axisLine: { | |
| ...COMMON_STYLES.axisLine, | |
| ...(option.yAxis && typeof option.yAxis === "object" && "axisLine" in option.yAxis | |
| ? (option.yAxis as any).axisLine | |
| : {}), | |
| }, | |
| splitLine: { | |
| ...COMMON_STYLES.splitLine, | |
| ...(option.yAxis && typeof option.yAxis === "object" && "splitLine" in option.yAxis | |
| ? (option.yAxis as any).splitLine | |
| : {}), | |
| }, | |
| axisLabel: { | |
| ...COMMON_STYLES.axisLabel, | |
| ...(option.yAxis && typeof option.yAxis === "object" && "axisLabel" in option.yAxis | |
| ? (option.yAxis as any).axisLabel | |
| : {}), | |
| }, | |
| nameTextStyle: { | |
| ...COMMON_STYLES.axisName, | |
| ...(option.yAxis && typeof option.yAxis === "object" && "nameTextStyle" in option.yAxis | |
| ? (option.yAxis as any).nameTextStyle | |
| : {}), | |
| }, | |
| ...option.yAxis, | |
| } | |
| : undefined |
| */ | ||
| export function createGradientColor( | ||
| colors: [string, string], | ||
| index: number, |
There was a problem hiding this comment.
createGradientColor(colors, index) declares an index parameter but never uses it. With Biome’s recommended rules enabled, this will be reported as an unused parameter and can fail lint. Either remove the parameter or rename it to _index (or actually use it) to satisfy linting.
| index: number, | |
| _index: number, |
| import { generateChartImage } from "../utils"; | ||
| import { | ||
| applyCommonStyles, | ||
| createGradientColor, |
There was a problem hiding this comment.
createGradientColor is imported from ../utils but never used in this file. With Biome’s recommended rules, unused imports can fail CI. Remove the import (or use it where intended).
| createGradientColor, |
| applyCommonStyles, | ||
| generateChartImage, | ||
| getAnimationConfig, | ||
| getColorPalette, |
There was a problem hiding this comment.
getColorPalette is imported from ../utils but never used in this file. With Biome’s recommended rules, unused imports can fail CI. Remove the import (or use it where intended).
| getColorPalette, |
| // 应用清新简约风格样式 | ||
| const styledOption = applyCommonStyles(echartsOption, theme); | ||
|
|
There was a problem hiding this comment.
This PR changes chart styling (colors, symbol sizes, tooltip/legend/title layout) for multiple tools, but no snapshot updates are included. The image snapshot tests under __tests__/tools/*.spec.ts (e.g. __tests__/tools/bar.spec.ts) are likely to fail until the golden images in __tests__/snapshots/*.png are regenerated/updated for the new look.
|
需要更新下单测截图。 |
…CLAUDE.md for project guidance