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

Hide parts of a dataset #404

Closed
rafalou38 opened this issue Nov 27, 2022 · 7 comments
Closed

Hide parts of a dataset #404

rafalou38 opened this issue Nov 27, 2022 · 7 comments
Labels

Comments

@rafalou38
Copy link

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
Copy link
Contributor

uhrjun commented Nov 28, 2022

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
Copy link
Contributor

uhrjun commented Dec 14, 2022

Successfully tracked this down to most definitely being a bug.

export function fillArray(array, count, element, start = false) {
if (!element) {
element = start ? array[0] : array[array.length - 1];
}
let fillerArray = new Array(Math.abs(count)).fill(element);
array = start ? fillerArray.concat(array) : array.concat(fillerArray);
return array;
}

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.

vals = fillArray(vals, datasetLength - vals.length, 0);

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 added a commit that referenced this issue Dec 14, 2022
Better null value handling. Add option for filling zero/null values to
maintain backwards compat.
@uhrjun uhrjun closed this as completed in 1b955a9 Dec 15, 2022
@uhrjun
Copy link
Contributor

uhrjun commented Dec 15, 2022

Validate this and let me know :D

@rafalou38
Copy link
Author

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
Copy link
Contributor

uhrjun commented Dec 21, 2022

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
Copy link
Author

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
Copy link
Contributor

uhrjun commented Jan 21, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants