fix(bar): fix bar chart bars overflowing the coordinate system #19973
Conversation
|
Thanks for your contribution! |
There was a problem hiding this comment.
This is a highly anticipated feature that has been requested by multiple users, so it's exciting to see this pull request.
Let's first clarify the behavior.
Without dataZoom
When dataZoom is not used, this pull request introduces a breaking change for bar series. According to the documentation, clip: true means: "Clip all the overflowed. With bar width kept." Therefore, we cannot consider the previous behavior a bug.
Bar: Clip all the overflowed. With bar width kept.
The English documentation might be a bit unclear. The Chinese version specifies that the bars are not clipped.
Recommendation
We can either:
- Maintain the behavior of
clip: trueand introduce another option likeclipModeor string values forclip: 'xxx'. While this may not be the perfect default solution, it does provide developers with a method to achieve their desired outcomes. - Clip bars outside the grid as the current implementation does, and introduce this breaking change in ECharts 6.0, which is expected to be released early next year. Special attention should be given to cases with a category axis with
boundaryGap: false, where the axis should shrink to ensure it won't be clipped. See the images below for reference.
Current Behavior
If we clip the bars as shown in the image below, the first and last bar will have half width, which is not expected.
Expected Behavior
If the axis can be shrunk and the grid width remains unchanged as shown below, then it won't be affected when clipped.
As illustrated, solution 2 requires more effort for you but has the potential to be more intuitive. Therefore, you may want to consider which approach we should take.
With dataZoom
If no breaking changes are considered, bars should never be clipped because neither clip: true nor any value of filterMode will result in the expected clipping behavior. If breaking changes are considered, please refer to the above notes on solution 2.
|
I appreciate the review, and can understand the concern about the breaking change. Despite my attempts, I couldn't find a case where Clipping everything outside the grid when |
|
@KurtGokhan Here is an example option = {
xAxis: {
min: 109
},
yAxis: {
type: 'value'
},
grid: {
left: 350
},
series: [
{
data: [120, 150, 80, 70, 110, 130],
type: 'bar',
clip: false
}
]
};
I agree with the saying that it's more intuitive to hide the two bars outside of the grid area. But for the bar at |
|
@Ovilia I see. What do you suggest? Maybe we can introduce a |
I'm making a breaking axis feature for ECharts 6.0 where bars get clipped by default. But I'm still working on it and not sure at this moment what it will ended up to be. I need to consider more carefully on this when I do with breaking axis, which may probably take another few months. So do you mind if we close this PR for now and see how it goes? |
|
@Ovilia ok, we can close this. But I need a fix for now because the bar chart clipping with the axis labels seems too wrong: I tried using a custom series. But I couldn't figure out how to do stacked bars with custom series. Any tips or resources you can guide me for doing that? |
|
Do you want to clip the left-most and right-most bars, or is this acceptable? |
|
Yes, I don't want the bars to go out of the grid. I also want it to work reliably with any axis type, i.e. time. |
|
Then I think custom series is a must. It would be challenging to implement stacking, but since stacking on time axis is also not supported for bar series, using custom series seems to be the only possible solution. |
It seems supported. I am using it and it works for me. The only limitation is that all series must have uniform data (all series must have values at same x axis values in the same order). |
|
Yes, it's supported, sorry I forgot. If you have no objection, perhaps we could close this PR and I will see what we can do when I complete the break axis feature. |
|
Tbh, my problem isn't solved. And I don't see how "breaking axes" solves the problem. But you can close the PR. |
|
It is a pity that this PR is closed without further development. I also need to clip the stacked bars to the grid. The proposed "strict" value for |
There was a problem hiding this comment.
It still does no fix the issue, whether a background is set to the bar serie.
A workaround would require to also clip the bar background in the edges and make not opaque the ones already far from them
Something like below:
.update(function (newIndex, oldIndex) {
var itemModel = data.getItemModel(newIndex);
var layout = getLayout[coord.type](data, newIndex, itemModel);
if (drawBackground) {
var isClipped = clip[coord.type](coordSysClipArea, layout);
var bgEl;
var bgSt;
var diff;
if (oldBgEls.length === 0) {
bgEl = createBackground(oldIndex);
}
else {
bgEl = oldBgEls[oldIndex];
bgSt = backgroundModel.getBarItemStyle();
if (isClipped) {
bgSt.opacity = 0;
}
bgEl.useStyle(bgSt);
// Only cartesian2d support borderRadius.
if (coord.type === 'cartesian2d') {
bgEl.setShape('r', barBorderRadius);
}
bgEls[newIndex] = bgEl;
}
var bgLayout = getLayout[coord.type](data, newIndex);
var shape = createBackgroundShape(isHorizontalOrRadial, bgLayout, coord);
if (coord.type === 'cartesian2d') {
if (shape.x < coordSysClipArea.x) {
diff = coordSysClipArea.x - shape.x;
shape.x += diff;
shape.width -= diff;
}
else if ((shape.x + shape.width) > (coordSysClipArea.x + coordSysClipArea.width)) {
diff = (shape.x + shape.width) - (coordSysClipArea.x + coordSysClipArea.width);
shape.width -= diff;
}
}
updateProps(bgEl, { shape: shape }, animationModel, newIndex);
}
var el = oldData.getItemGraphicEl(oldIndex);
if (!data.hasValue(newIndex)) {
group.remove(el);
return;
}
if (needsClip) {
var isClipped = clip[coord.type](coordSysClipArea, layout);
if (isClipped) {
group.remove(el);
return;
}
}
....
Brief Information
This pull request is in the type of:
What does this PR do?
Bar chart clip option now clips the bar at the edges of the coordinate system so that it doesn't overflow.
Fixed issues
Fixes #19972
Details
See #19972 for detailed info and additional links.
Before: What was the problem?
See example
After: How does it behave after the fixing?
bar-cliptest covers this case.There is also a test with
dataZoom.filterMode:'none'Document Info
Misc
ZRender Changes
Related test cases or examples to use the new APIs
N.A.
Others
Merging options
Other information
Here is the diff