fix(huicharts): fix the double-layer donut chart error in charts #4029
Conversation
WalkthroughModified the ring chart's Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vue/src/huicharts/huicharts-ring/src/ring.ts (1)
263-263: MissinginnerDataparameter ingetLegendcall.The
getLegendfunction expectsinnerDatain its parameters (line 198) and uses it whenlevelis defined (line 222). However,innerDatais not passed in the call at line 263, even though it's available in scope (defined at line 251). This results inlegend.databeingundefinedfor multi-layer charts.Suggested fix
- const ichartLegend = getLegend({ legendVisible, dimension, innerRows, legendLimit, level, limitShowNum }) + const ichartLegend = getLegend({ legendVisible, dimension, innerRows, legendLimit, level, limitShowNum, innerData })
🤖 Fix all issues with AI agents
In `@packages/vue/src/huicharts/huicharts-ring/src/ring.ts`:
- Line 194: The call to getLegend is missing innerData, so when level is present
the function destructures innerData and ends up with ichartLegend.data
undefined; update the getLegend invocation (the one that builds ichartLegend) to
include innerData in its argument object (e.g., pass innerData along with
legendVisible, dimension, innerRows, legendLimit, level, limitShowNum) so
getLegend receives the full dataset used by getLimitData/series for multi-layer
ring charts.
🧹 Nitpick comments (1)
packages/vue/src/huicharts/huicharts-ring/src/ring.ts (1)
134-195: Consider renaminggetDataOrSeriesfor clarity.The function name
getDataOrSeriessuggests it returns either data or series (exclusive OR), but with the fix it now returns both properties whenlevelFlagis true. Consider renaming to something likegetChartDataorgetDataAndSeriesto better reflect its behavior.
| } | ||
|
|
||
| return levelFlag ? { series } : { data: getLimitData(innerData) } | ||
| return levelFlag ? { series, data: getLimitData(innerData) } : { data: getLimitData(innerData) } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the actual file
cat -n packages/vue/src/huicharts/huicharts-ring/src/ring.ts | sed -n '190,200p'Repository: opentiny/tiny-vue
Length of output: 178
🏁 Script executed:
# Search for ichart library imports and type definitions
rg -n 'import.*ichart|from.*ichart' packages/vue --type=ts --type=js | head -20Repository: opentiny/tiny-vue
Length of output: 2178
🏁 Script executed:
# Look for type definitions or interfaces related to ichart options
fd -e d.ts -e ts -path '*/node_modules/@*ichart*' 2>/dev/null | head -10Repository: opentiny/tiny-vue
Length of output: 43
🏁 Script executed:
# Search for ichart usage with series and data properties
rg -n -C3 'series\s*:.*data\s*:' packages/vue --type=ts --type=js | head -40Repository: opentiny/tiny-vue
Length of output: 43
🏁 Script executed:
# Check how getLegend function is used and what it expects
rg -n -B5 -A5 'getLegend' packages/vue/src/huicharts/huicharts-ring/src/ring.tsRepository: opentiny/tiny-vue
Length of output: 796
🏁 Script executed:
# Look for other ring chart implementations to understand the pattern
find packages/vue -name "*.ts" -path "*/ring*" -o -name "*.ts" -path "*/donut*" | xargs rg -l "series.*data" 2>/dev/nullRepository: opentiny/tiny-vue
Length of output: 114
🏁 Script executed:
# Check the full getLegend function body to see if innerData is used
cat -n packages/vue/src/huicharts/huicharts-ring/src/ring.ts | sed -n '197,240p'Repository: opentiny/tiny-vue
Length of output: 1573
🏁 Script executed:
# Check how ichartLegend and dataOrSeries are merged into ichartOption
cat -n packages/vue/src/huicharts/huicharts-ring/src/ring.ts | sed -n '270,290p'Repository: opentiny/tiny-vue
Length of output: 687
🏁 Script executed:
# Search for other chart types that use level to see if they also return both series and data
rg -n 'levelFlag.*series.*data' packages/vue --type=ts | head -20Repository: opentiny/tiny-vue
Length of output: 341
🏁 Script executed:
# Look for tests or examples that show how multi-layer ring charts are used
find packages/vue -name "*.test.ts" -o -name "*.spec.ts" | xargs rg -l "ring\|donut" 2>/dev/null | head -5Repository: opentiny/tiny-vue
Length of output: 43
Pass innerData to getLegend function when calling at line 263.
The change at line 194 is correct—ichart requires both series (for layer-specific data) and data (for the full dataset) in multi-layer ring charts. However, there is a critical bug: the getLegend function (line 198) destructures innerData and uses it when level is present (line 222), but the function call at line 263 does not pass innerData as a parameter. This causes ichartLegend.data to be undefined for multi-layer charts, breaking legend rendering.
Update line 263 to:
const ichartLegend = getLegend({ legendVisible, dimension, innerRows, legendLimit, level, limitShowNum, innerData })
🤖 Prompt for AI Agents
In `@packages/vue/src/huicharts/huicharts-ring/src/ring.ts` at line 194, The call
to getLegend is missing innerData, so when level is present the function
destructures innerData and ends up with ichartLegend.data undefined; update the
getLegend invocation (the one that builds ichartLegend) to include innerData in
its argument object (e.g., pass innerData along with legendVisible, dimension,
innerRows, legendLimit, level, limitShowNum) so getLegend receives the full
dataset used by getLimitData/series for multi-layer ring charts.
1 participant
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.