frappe / charts

Simple, responsive, modern SVG Charts with zero dependencies
https://frappe.io/charts
MIT License
14.94k stars 717 forks source link

Hide parts of a dataset #404

Closed rafalou38 closed 1 year ago

rafalou38 commented 1 year ago

Expected Behaviour

When a value on a dataset is null or a dataset is incomplete, a gap appears at that place.

Maybe a setting could be added to the dataset settigns for example:

{
    values: [1,5,6,7,2],
    name: "example",
    type: "line",
    continuous: false
}

Actual Behaviour

The line continues straight, ignoring the missing data.

Steps to Reproduce:

Create an axis chart with two datasets, one smaller than the other, and see the issue:

Here, I would like to have the blue line stop when I haven't got any more data (after 28/11/2022). image

Frappe Charts version: 1.6.2 Codepen / Codesandbox: https://codepen.io/rafalou38/pen/rNKKvOw

uhrjun commented 1 year ago

When a value on a dataset is null or a dataset is incomplete, a gap appears at that place.

I am marking this as a feature request and looking into it but I'm leaning towards this being a simple oversight. Possibly should be the default behavior. Thanks for filing the issue.

NOTE: This is also replicated in bar charts so look into AxisChart.js

uhrjun commented 1 year ago

Successfully tracked this down to most definitely being a bug.

https://github.com/frappe/charts/blob/9dce5cf5a86832545fd157fcdc4c36757aa19eb2/src/js/utils/helpers.js#L49-L56

The function for fillArray(). Does not correctly validate the element argument. Check if (!element)

Causing it to fail when it's called here when initializing an AxisChart with a value of 0. Since 0 is a falsy in JS it's no different than false or undefined causing if (!element) to be true always. Whereas, I infer the desired behaviour would be it actually being the value of 0 and not a falsy.

Effectively killing the desired functionality. https://github.com/frappe/charts/blob/9dce5cf5a86832545fd157fcdc4c36757aa19eb2/src/js/utils/axis-chart-utils.js#L39

As for the fix right now. I am just going to patch the correct element type validation. But that breaks the animation lifecycle entirely which is rather hacky so the animate option is going to default to false from this point forward.

uhrjun commented 1 year ago

Validate this and let me know :D

rafalou38 commented 1 year ago

I installed from the GitHub

yarn add https://github.com/frappe/charts  
cd node_modules
npm i
npm run build

But I can't get it t work, I still get the continuous line 😕

uhrjun commented 1 year ago

Yeah sorry, I should've expanded on the implementation more. Also need to update the docs. So the value continuous defaults to true, as to not break the existing configs.

Here's how to use the continuous property to not fill the remaining dataset

const data = {
  labels: ["1", "2", "3", "4", "5", "6", "7"],
  datasets: [
    {
      name: "Another Set",
      chartType: "line",
      values: [10, 20, 25],
    },
  ],
};

const chart = new frappe.Chart("#chart",
  title: "My Awesome Chart",
  data: data,
  type: "line",
  height: 800
  continuous: 0,
});
rafalou38 commented 1 year ago

Hi,

I tried with continuous, but it is still not working as expected.

Image

I would have liked the dots to stop: image (obtained with the inspector)

Ps: I had a hard time getting the latest version to load. Could you publish a release after that?

uhrjun commented 1 year ago

Yes this is the expected behavior right now. Need to review #243 or something similar for null value support in line charts. This is a stop gap solution right now to draw 0s. Bar charts exhibit the desired behavior.

Ps: I had a hard time getting the latest version to load. Could you publish a release after that?

Yeah thats also work that needs to be done. New release is going to most likely be after I'm done updating the docs and adding CI