Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] BarChart does not respect yAxis.min when all data is below min (and max is undefined) #19187

Open
tmtron opened this issue Oct 10, 2023 · 7 comments
Labels
bug difficulty: easy Issues that can be fixed more easily than the average. en This issue is in English topic: bar

Comments

@tmtron
Copy link

tmtron commented Oct 10, 2023

Version

5.4.3

Link to Minimal Reproduction

https://echarts.apache.org/examples/en/editor.html?code=PYBwLglsB2AEC8sDeAoWsAeBBDEDOAXMmurGAJ4gCmRA5AMYCGYVA5sAE7m0A0J6AE2aMiAbVpZaAXRIBfPunI58RVKTKUasWgDdGAGwCuVXv1gBbCNCIBWOQth4qHCFUKxRZtethCwIjwBaAEYABlCpB3UKajoAI0YOWjNZEhlZAG4gA

Steps to Reproduce

  • yAxis.min=5
  • we have only 1 bar with value -100

Current Behavior

The bar-chart ignores the yAxis.min setting (5) and instead seems to use the auto-calculated value (100)

Expected Behavior

  • The yAxis minimum value should be 5, as specified

Environment

- OS: Windows 11
- Browser: Google Chrome
- Framework: none

Any additional comments?

it works as expected when

  • we set an explicit max: e.g. 6: Example
  • we use -5 as yAxis.min, Example
  • when we have some data that is higher than yAxis.min: example
@tmtron tmtron added the bug label Oct 10, 2023
@echarts-bot echarts-bot bot added en This issue is in English pending We are not sure about whether this is a bug/new feature. labels Oct 10, 2023
@helgasoft
Copy link

A very thorough analysis and bug confirmed.
OTOH, what would be the use of such chart ?

option = {
  xAxis: { data: ['A'] },  yAxis: { min: 5 },
  series: [ { data: [-100], type: 'bar' }]
};

@tmtron
Copy link
Author

tmtron commented Oct 12, 2023

Our use-case is that customers can create custom dashboards and add many charts side-by-side or below each other.
In this case they have 3 bar-charts side-by-side and want to compare data from 3 different devices. All devices have 3 channels 'A', 'B', 'C' which are expected to return data that is higher than a certain offset: e.g. 220.
In this case it makes sense to set the yAxis.min to 220 so that you can see small differences exactly, e.g. when the data are: 220, 225, 223. And of course, you want to set the same min to all 3 charts, so that you can easily compare them.
When a channel has an error, it will return a negative number: e.g. -1, -2, etc.

And now we have this special case, where all 3 channels return an error and in this case, the yAxis scale is different:

2023-10-12_08h55_23

The biggest issue with that is that the customers will check the config of chart 2 and see that they have set the min correctly, but the chart is not okay. This causes confustion and a lot of wasted support time on our side.

BTW: I just noticed that the label of bar B in the 3rd chart is missing: I created a separate issue for that: #19196

@Ovilia Ovilia added the difficulty: easy Issues that can be fixed more easily than the average. label Oct 12, 2023
@echarts-bot
Copy link

echarts-bot bot commented Oct 12, 2023

This issue is labeled with difficulty: easy.
@tmtron Would you like to debug it by yourself? This is a quicker way to get your problem fixed. Or you may wait for the community to fix.

Please have a look at How to debug ECharts if you'd like to give a try. 🤓

@Ovilia Ovilia added topic: bar and removed pending We are not sure about whether this is a bug/new feature. labels Oct 12, 2023
@helgasoft
Copy link

Much better, @tmtron, a complete description of the problem. Now it becomes evident that it's a design issue.
Values between 0 and 220 are irrelevant, thus the threshold(220 ) has to be substracted from data values. Then yAxis needs to be re-labeled ... et voila.
Demo Code
image

@tmtron
Copy link
Author

tmtron commented Oct 13, 2023

@helgasoft Your proposal is for sure nice, but just a workaround for the bug.
And you must also consider that we don't just create a fixed chart for one customer, so we would need to change the config dialouge to let the customer define if they want that "error" display or not.
We definitely don't want to add complexity to our input dialogs or source code just to make up for bugs in the chart library.

BTW: an easier workaround would be to simply check if all data are below 0 and then set max to the threshold plus some constant, e.g. 220+10 = 230: Example

@soniyaprasad77
Copy link

@tmtron
Here's a revised code snippet implementing the dynamic adjustment of yAxis.max based on whether all data points are below zero:

const data = [-1, -2, -3]; // Example data
const threshold = 220;

// Check if all data points are below zero
const allBelowZero = data.every(value => value < 0);

// Dynamically set yAxis.max based on the condition
const maxY = allBelowZero ? threshold + 10 : null;

const option = {
xAxis: {
type: 'category',
data: ['A', 'B', 'C'] // Example categories
},
yAxis: {
type: 'value',
min: threshold,
max: maxY // Set to threshold + 10 if all data points are below zero
},
series: [
{
data: data,
type: 'bar'
}
]
};

This adjustment allows for a more dynamic setting of the yAxis.max based on the data provided, ensuring the chart accommodates the range while avoiding hard-coded values. It keeps the configuration adaptable and avoids unnecessary complexity in the input dialogs or source code.

@mertokten
Copy link

@tmtron
I have added this code snippet after line 235 in the scaleRawExtentInfo.ts file and it seemed to fix the current bug. However I am not sure whether it breaks something when maxFixed and minFixed are set like this.

    if (!maxFixed && minFixed) {
        if (min > max) {
            max = min + 10;
            maxFixed = true;
        }
    } else if (!minFixed && maxFixed) {
        if (max < min) {
            min = max - 10;
            minFixed = true;
        }
    }

I am currently testing if it breaks anything else by checking with the other test html files.

How does this solution look for you, I would appreciate the feedback.

mertokten added a commit to mertokten/echarts that referenced this issue Dec 13, 2023
@mertokten mertokten mentioned this issue Dec 13, 2023
8 tasks
mertokten added a commit to mertokten/echarts that referenced this issue Dec 13, 2023
mertokten added a commit to mertokten/echarts that referenced this issue Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug difficulty: easy Issues that can be fixed more easily than the average. en This issue is in English topic: bar
Projects
None yet
Development

No branches or pull requests

5 participants