fix: fix axis label configuration for ECharts with additional properties #39803
Conversation
WalkthroughThis pull request updates the axis label configuration for chart widgets. In the X-axis builder, the method now returns additional properties—including width, overflow, hideOverlap, and interval—based on the label orientation. In the Y-axis builder, the overflow truncation property has been removed, which may affect label rendering when the label size exceeds the allocated width. No changes were made to public API declarations. Changes
Sequence Diagram(s)sequenceDiagram
participant Widget as ChartWidget
participant XAxis as EChartsXAxisLayoutBuilder
Widget->>XAxis: call axisLabelConfig(labelOrientation, allocatedXAxisHeight)
alt labelOrientation == AUTO
XAxis-->>Widget: returns { width: allocatedXAxisHeight, overflow, hideOverlap, interval }
else
XAxis-->>Widget: returns { hideOverlap: true, interval: 0, ...existingProps }
end
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🔇 Additional comments (2)
✨ 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. 🪧 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
Documentation and Community
|
|
Hey @rahulbarwal I have raised the pr, can you please check it. |
vivek-appsmith
added
the
Widgets Product
| nameGap: this.nameGap, | ||
| axisLabel: { | ||
| width: this.labelsWidth, | ||
| overflow: "truncate", |
There was a problem hiding this comment.
We should debug why the label width is incorrect. Calculating the correct width for labels and the chart area should fix the bug. Removing truncate will make other use cases fail.
Same approach for x label truncation issue too.
There was a problem hiding this comment.
Once the fix is done, I would like to request you to test the client unit test cases pass as well since there are a lot of unit test cases for the chart widget which check various scenarios which should pass after the changes. Thank you!
There was a problem hiding this comment.
@rajatagrawal
thanks for pointing out the real issue and for x axis we are having that issue in auto orientation only so we need to add the truncate properties in it and else I am debugging what could be the problem as you are correct about not removing truncate as it will affect the other edge cases
|
Hey @rajatagrawal, since this is assigned to you, I wanted to share a few observations:
although I am still working on it |
|
hey @rajatagrawal , I think using a formatter would be a better approach as the width is dynamic and we are calculating it on the basis of text width and that's why even if we truncate on the basis of this dynamic width we will still face inconsistency for bigger numbers, let me know what do you think about this aproach |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
This PR has been closed because of inactivity. |
3 participants
Fixes #38974
Fixes the issue where Y-axis labels were getting truncated, as mentioned in the issue.
Also resolves the problem of X-axis labels not being truncated when displaying large values. The fix ensures that truncation occurs in two cases:
When the orientation is set to "auto," truncation happens for relatively larger values.
For other orientations, truncation follows the same width constraints as before.
Automation
/ok-to-test tags=""
🔍 Cypress test results
Caution
If you modify the content in this section, you are likely to disrupt the CI result for your PR.
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit