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

Line chart doesn't redraw correctly after removeEntry() #2411

Closed
jscalo opened this issue May 3, 2017 · 6 comments
Closed

Line chart doesn't redraw correctly after removeEntry() #2411

jscalo opened this issue May 3, 2017 · 6 comments

Comments

@jscalo
Copy link

jscalo commented May 3, 2017

If you add a few entries to a line chart and have for e.g. this:

simulator screen shot may 3 2017 2 33 55 pm

Then remove the entry at index 2 ("83") and you get something like this:

simulator screen shot may 3 2017 2 34 02 pm

Clearly the chart isn't redrawing correctly. The old grid line is still there and the space where the old value was didn't condense. Here's a sample project to demonstrate:

ChartsRemoveBug.zip

(You'll need to add the Charts directory to the top level…)

This sounds very similar to #745 which was closed after the bug mysteriously stopped happening to the originator.

@liuxuan30
Copy link
Member

liuxuan30 commented May 5, 2017

You call removeEntry at data set level, which seems not correct, try call chartData.removeEntry(xValue: Double, dataSetIndex: Int) -> Bool
It will recalculate the min/max value at data level to adjust the axis if necessary.

@jscalo
Copy link
Author

jscalo commented May 5, 2017

Hi Xuan,

I get the exact same behavior when using chartData.removeEntry(xValue: Double, dataSetIndex: Int) -> Bool.

Here's the exact code:

self.chart.data?.removeEntry(xValue: Double(self.removeTextField.text!)!, dataSetIndex: 0)
self.chart.data?.notifyDataChanged()
self.chart.notifyDataSetChanged()

Can you please reopen until we both agree this is addressed?

As an aside, isn't

func removeEntry(xValue: Double, dataSetIndex: Int) -> Bool

wrongly declared? If xValue is intended to be an index then a) it should be named xIndex and b) it should be an Int.

Thanks!

@liuxuan30
Copy link
Member

Since chart 3.0, x axis is same as y axis almost. So taking double is correct. The question is, has it remove the entry you want?
What does not work? It's supposed to recalculate the axis range and render the line again. If the entry you removed is the not min/max value, it should have no impact then.

@liuxuan30 liuxuan30 reopened this May 8, 2017
@jscalo
Copy link
Author

jscalo commented May 8, 2017

What does not work?

The issue is exactly as I originally described it— the point is removed but the graph is not redrawn. The old grid line is still there and the x axis doesn't "squeeze in". I attached a sample project to the original description so you can reproduce it easily.

@liuxuan30
Copy link
Member

liuxuan30 commented May 9, 2017

If you are talking about your screenshots, I'd say it's expected from my side. The '83' point is not connected, and 96 directly connected to whatever '8' is on the right edge.

About the x axis 'squeeze in', I don't see any reason for it, as the x axis range is not changed, right? You removed one entry, whose x value is neither min nor max, so the x axis should not change.

You seem take x axis as indexed based axis just like chart 2.x, but in chart 3.0, the x axis behaves like y axis. You can -1 for the rest of your entries' x value, so it will squeeze in I suppose.

@jscalo
Copy link
Author

jscalo commented May 9, 2017

Yeah, the x values are indexed in this case so I'd need to go through all the subsequent x values and decrement them by 1 to get that effect. That makes sense. Sorry for misunderstanding!

@jscalo jscalo closed this as completed May 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants