fix(JadeJueChart): use the axisTick.customValues property to divide … #153
Conversation
WalkthroughThis pull request updates the axis label formatting logic for the JadeJueChart. In Changes
Sequence Diagram(s)sequenceDiagram
participant C as Chart Component
participant H as handleLabelFormatter
participant D as DefaultFormatter
participant A as AngleAxis Config
C->>H: Call handleLabelFormatter(options, baseOpt)
alt Formatter is "percent" or undefined
H->>D: Invoke defaultFormatter(value, angleAxis.sum)
else Formatter is "number"
H->>H: Use native number formatting
end
H->>A: Compute customValues using splitNumber
A-->>H: Return updated angleAxis settings
H-->>C: Return updated chart options
Possibly related PRs
Poem
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/JadeJueChart/labelFormatter.jsOops! Something went wrong! :( ESLint: 8.56.0 ReferenceError: require is not defined in ES module scope, you can use import instead src/option/config/angleAxis/index.jsOops! Something went wrong! :( ESLint: 8.56.0 ReferenceError: require is not defined in ES module scope, you can use import instead ✨ 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/JadeJueChart/labelFormatter.js(2 hunks)src/option/config/angleAxis/index.js(1 hunks)
🔇 Additional comments (3)
src/option/config/angleAxis/index.js (1)
20-20: LGTM! Setting splitNumber for JadeJueChart.The addition of
splitNumber = 5aligns with the PR objective to divide the scale into five equal parts. This value is correctly used by the new customValues calculation in labelFormatter.js.src/components/JadeJueChart/labelFormatter.js (2)
114-114: LGTM! Clear and concise percentage formatter.The defaultFormatter function correctly calculates percentage values based on angleAxis.sum.
117-117: LGTM! Simplified formatter selection logic.The code now consistently uses defaultFormatter for both 'percent' and undefined cases, improving maintainability.
Also applies to: 123-123
| // 使用customValues属性(echarts5.5.1以上版本才支持)将刻度均分为5份。 | ||
| const customValues = [0]; | ||
| const interval = angleAxis.splitNumber || 5; | ||
| const averageVal = angleAxis.sum / interval; | ||
| for (let i = 0; i < interval; i++) { | ||
| customValues.push(customValues[customValues.length - 1] + averageVal); | ||
| } | ||
| angleAxis.axisTick.customValues = customValues; | ||
| angleAxis.axisLabel.customValues = customValues; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify ECharts version compatibility and add input validation.
The implementation correctly divides the scale into equal parts using customValues. However, there are two considerations:
- The comment indicates this feature requires ECharts 5.5.1+, but there's no version check.
- The splitNumber value should be validated to prevent potential issues.
Consider adding version validation and input checks:
// 使用customValues属性(echarts5.5.1以上版本才支持)将刻度均分为5份。
+const ECHARTS_MIN_VERSION = '5.5.1';
+const MIN_SPLIT_NUMBER = 2;
+const MAX_SPLIT_NUMBER = 10;
+
+// Ensure splitNumber is within reasonable bounds
+const validSplitNumber = Math.max(MIN_SPLIT_NUMBER,
+ Math.min(MAX_SPLIT_NUMBER, angleAxis.splitNumber || 5));
+
const customValues = [0];
-const interval = angleAxis.splitNumber || 5;
+const interval = validSplitNumber;
const averageVal = angleAxis.sum / interval;Please verify the ECharts version requirement:
🌐 Web query:
What is the minimum ECharts version that supports axisTick.customValues?
💡 Result:
The axisTick.customValues feature for specifying custom tick positions was introduced in Apache ECharts 5.0.0. This version marked a major upgrade with significant API improvements and new customization capabilities[5][9].
Key points:
- Version 5.x introduced enhanced axis configuration options as part of its focus on flexibility and developer experience[5].
- The
customValuesproperty allows explicit definition of tick locations instead of relying on automatic calculation[9]. - Earlier versions (4.x and below) do not support this feature natively, requiring workarounds for custom tick positioning[10].
For implementation details:
yAxis: {
type: 'value',
axisTick: {
customValues: [0, 25, 50, 75, 100] // Explicitly set tick positions
}
}Citations:
- 1: https://apache.github.io/echarts-handbook/en/concepts/axis/
- 2: https://www.knime.com/blog/apache-echarts
- 3: https://echarts.apache.org/handbook/en/basics/release-note/5-3-0/
- 4: [Bug] : xaxis timeseries chart axis values overlapping to each other for small width apache/echarts#19504
- 5: https://echarts.apache.org/handbook/en/basics/release-note/v5-upgrade-guide/
- 6: https://echarts.apache.org/en/faq.html
- 7: https://community.openhab.org/t/custom-widget-echarts-bar/160978
- 8: Data Zoom min and Max Values are not correct when yAxis scale option true. apache/echarts#19358
- 9: https://echarts.apache.org/en/option.html
- 10: min/max on yAxis for bar charts, makes the bars go outside limits apache/echarts#8841
ECharts Version Mismatch—Update Version Check and Validate Input
The research shows that the axisTick.customValues feature was introduced in Apache ECharts 5.0.0, not 5.5.1 as indicated in the comment. The suggested validation for the splitNumber is still a good improvement, but the version requirement should be updated accordingly.
- In file
src/components/JadeJueChart/labelFormatter.js(lines 132-140), update the version constant (and comment) from 5.5.1 to 5.0.0. - Ensure that any added validation of
angleAxis.splitNumberaligns with acceptable input ranges.
Committable suggestion skipped: line range outside the PR's diff.
…the scale into 5 equal parts and hide any excess scale lines
lingdan33
changed the title
…the scale into 5 equal parts and hide any excess scale lines
Summary by CodeRabbit