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

ChartsDemo-Swift crashes #3327

Closed
valeriyvan opened this issue Mar 11, 2018 · 6 comments
Closed

ChartsDemo-Swift crashes #3327

valeriyvan opened this issue Mar 11, 2018 · 6 comments
Labels

Comments

@valeriyvan
Copy link
Contributor

ChartsDemo-Swift crashes when in Line Chart in option choose Toggle Data option and then choose, Toggle Cubic option.

@liuxuan30 liuxuan30 added the bug label Mar 11, 2018
@liuxuan30
Copy link
Member

liuxuan30 commented Mar 12, 2018

That's because you niled out the chartData and then try to unwrap it:

        case .toggleCubic:
            for set in chartView.data!.dataSets as! [LineChartDataSet] {
                set.mode = (set.mode == .cubicBezier) ? .linear : .cubicBezier
            }

@jjatie this is no big deal. But I checked many of the view controllers uses chartView.data!.dataSets, do you have any smart fix for this? Or we have to fix it one by one.

One option would be add a check before each override func optionTapped(_ option: Option), or we have a if let before each for set in chartView.data!.dataSets

@jjatie
Copy link
Collaborator

jjatie commented Mar 12, 2018

Looks like we'll need to guard each handleOption implementation.

@jjatie
Copy link
Collaborator

jjatie commented Mar 14, 2018

@liuxuan30 @valeriyvan I've thought about this some more. It's easy to imagine a case where a user wants to temporarily hide a given ChartData. It seems to me that ChartData should have an isEnabled property or something. If isEnabled == false then it's data sets are not drawn.z

Once this implemented, the demos should not be setting the data to nil, but simply toggling chartData.isEnabled

@valeriyvan
Copy link
Contributor Author

Does it sound like data should’t be optional anymore?

@jjatie
Copy link
Collaborator

jjatie commented Mar 15, 2018

That's a different discussion, but I've been thinking yes.

@jjatie
Copy link
Collaborator

jjatie commented Apr 1, 2018

This seems like a corner case and optionality of many properties are changing in 4.0. As we have limited resources to keep making changes for non-critical areas, I am closing this. After 4.0 is released, we will have more time to reevaluate issues like this.

@jjatie jjatie closed this as completed Apr 1, 2018
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

3 participants