patrykandpatrick / vico

A light and extensible chart library for Android.
https://patrykandpatrick.com/vico/wiki
Apache License 2.0
2.11k stars 127 forks source link

Dynamically Removing a LineCartesianLayer Crashes #914

Open Tyler-Lopez opened 12 hours ago

Tyler-Lopez commented 12 hours ago

How to reproduce

Background

I posted this same issue before as a Discussion Question. A point was made which at the time, made sense to me. This point was that, regardless of the error, the approach I was taking was unnecessary - I should not be dynamically adding or removing LineCartesianLayer.

However, I no longer feel this is unnecessary. So, I am opening this as a Bug - I think that is more correct than the original Question.

The problem is that you get the following crash when you dynamically remove a LineCartesianLayer.

Crash: https://gist.github.com/Tyler-Lopez/b8feff82b168366f3b1e5d3b2cce66f6

java.lang.ClassCastException: com.patrykandpatrick.vico.core.cartesian.layer.LineCartesianLayer cannot be cast to com.patrykandpatrick.vico.core.cartesian.CartesianChart

Reproducible Minimal Example: https://gist.github.com/Tyler-Lopez/0358e74977973034b7559ae7a172114b**

Motivation For Why This Should Work

Other Note

I would love if, when cartesian layers were added & removed, they did not "reset" the chart. That is one really nice benefit to only use a single LineCartesianLayer: when you add and remove series from it, it doesn't reset the chart, it just animates in or out that one series.

Demo

https://github.com/user-attachments/assets/4d256212-0c18-42ea-a178-687e242fe652

Observed behavior

Removing (but not adding) a LineCartesianLayer crashes.

Expected behavior

Removing a LineCartesianLayer should not crash, like how the ColumnCartesianLayer removal doesn't crash.

Vico version(s)

2.0.0-beta.1

Android version(s)

33

Additional information

No response

Tyler-Lopez commented 12 hours ago

Oh - and I want to mention: that my minimum reproducible example is extremely simplified. I want to get ahead of a previous comment/question which was:

Why is this necessary? The layer won't be drawn if there's no data for it.

So, why is it necessary to remove a line cartesian layer dynamically?

It is necessary if the inputs themselves are completely dynamic. In my minimal example, they are over-simplified and not dynamic - so you could just leave the layers without removing them.

However, imagine if a parameter to the Composable was a list of "data series", like List<Triple<Double, Double, DataSeriesType>> where DataSeriesType could be a COLUMN or LINE.

Rather than, within the Composable, deciding what to show, it is state-hoisted and provided to it.

In that case, when you update the list to remove a line, you have no choice but to remove the layer with it.

If this is confusing just let me know and I can write out a full example along these lines.