fix(AreaChart): update markLine specification #123
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
src/components/LineChart/handleSeries.jsOops! Something went wrong! :( ESLint: 8.56.0 ReferenceError: require is not defined in ES module scope, you can use import instead WalkthroughThe pull request introduces a new Changes
Possibly related PRs
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/LineChart/handleSeries.js (1)
Line range hint
82-117: Consider refactoring the isTopOrBottom function for better maintainability.The function has grown to handle multiple responsibilities including position, labels, colors, and line styles. Consider breaking it down into smaller, focused functions:
-function isTopOrBottom(markLine, seriesUnit, flag) { +function getMarkLinePosition(markLine, flag) { let position = ''; let markLinePosition = ''; let markLineLabel = ''; let markLineColor = ''; let markLinelLineStyle; - let markLineData = {}; + if (flag === 'top') { position = markLine.top; markLinePosition = markLine.topPosition markLineLabel = markLine.topLabel; markLineColor = markLine.topColor; markLinelLineStyle = markLine.topLine } else { position = markLine.bottom; markLinePosition = markLine.bottomPosition markLineLabel = markLine.bottomLabel; markLineColor = markLine.bottomColor markLinelLineStyle = markLine.bottomLine } + return { position, markLinePosition, markLineLabel, markLineColor, markLinelLineStyle }; +} + +function createMarkLineData(position) { + if (isString(position)) { + return { type: position }; + } + return { yAxis: position }; +} + +function configureMarkLineLabel(markLineData, markLinePosition, markLineLabel, markLineColor) { markLineData.label = { show: false, position: 'insideEndTop', lineHeight: Theme.config.markLineLabelLineHeight }; if (markLinePosition) { markLineData.label.position = markLinePosition; } if (markLineLabel) { markLineData.label.show = true; markLineData.label.color = 'inherit'; markLineData.label.formatter = markLineLabel; } if (markLineColor === 'auto') { markLineColor = undefined; } if (markLineColor) { markLineData.label.color = markLineColor; } +} + +function isTopOrBottom(markLine, seriesUnit, flag) { + const { + position, + markLinePosition, + markLineLabel, + markLineColor, + markLinelLineStyle + } = getMarkLinePosition(markLine, flag); + + const markLineData = createMarkLineData(position); + + configureMarkLineLabel(markLineData, markLinePosition, markLineLabel, markLineColor); + markLineData.lineStyle = {}; if (!markLineColor) { setThresholdMarkLineLabel(markLineData); } if (markLineColor) { markLineData.lineStyle.color = markLineColor; } if (markLinelLineStyle === false) { markLineData.lineStyle.color = chartToken.color; } seriesUnit.markLine.data.push(markLineData); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/components/LineChart/handleSeries.js(1 hunks)src/feature/token/factory/getModelToken.js(2 hunks)src/feature/token/theme/cloud/getAliasToken.js(1 hunks)src/feature/token/theme/hdesign/getAliasToken.js(1 hunks)src/feature/token/theme/ict/getAliasToken.js(1 hunks)src/option/config/mark/index.js(1 hunks)
🔇 Additional comments (6)
src/feature/token/theme/cloud/getAliasToken.js (1)
186-187: LGTM! Consistent theme token implementation.The addition of
labelLineHeightproperty withsize5xvalue aligns with the theme configuration standards.src/feature/token/theme/hdesign/getAliasToken.js (1)
190-191: LGTM! Consistent theme token implementation.The addition of
labelLineHeightproperty maintains consistency with other theme configurations.src/feature/token/theme/ict/getAliasToken.js (1)
185-186: LGTM! Consistent theme token implementation.The addition of
labelLineHeightproperty completes the standardization across all theme configurations.src/components/LineChart/handleSeries.js (1)
99-99: LGTM! Correct implementation of the line height configuration.The addition of
lineHeightproperty correctly utilizes the theme configuration.src/feature/token/factory/getModelToken.js (1)
70-70: LGTM! Clean implementation of the line height configuration.The addition of
labelLineHeightand its usage inmarkLineLabelLineHeightfollows the existing pattern and properly implements the line height configuration for mark lines.Also applies to: 327-328
src/option/config/mark/index.js (1)
28-28: LGTM! Proper integration of line height configuration.The addition of
lineHeightproperty is consistent with the token changes and correctly references the theme configuration.The AI summary indicates removal of several label properties (backgroundColor, borderColor, borderWidth, borderRadius, padding). Let's verify if these styles are now handled elsewhere:
✅ Verification successful
Verified: Style properties properly moved to theme system
The removed style properties (backgroundColor, borderColor, etc.) are now correctly handled through theme tokens in
getModelToken.jsand dynamic styling inhandleSeries.js. This is an improvement in the architecture.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any alternative styling of mark line labels rg -l "markLine.*label.*(\{|backgroundColor|borderColor|borderWidth|borderRadius|padding)" --type js # Check for any theme configurations that might be handling these styles rg -l "markLineLabelBg|markLineLabelBorder" --type jsLength of output: 242
Script:
#!/bin/bash # Check mark line label styling in handleSeries.js rg "markLine.*label.*\{" -A 10 src/components/LineChart/handleSeries.js # Check theme token definitions in getModelToken.js rg "markLine.*Label" -A 5 src/feature/token/factory/getModelToken.jsLength of output: 1061
Summary by CodeRabbit
New Features
labelLineHeightconfiguration option for mark line labels across multiple themesRefactor