imaNNeo / fl_chart

FL Chart is a highly customizable Flutter chart library that supports Line Chart, Bar Chart, Pie Chart, Scatter Chart, and Radar Chart.
https://flchart.dev
MIT License
6.87k stars 1.78k forks source link

Missing FlTouchInputEnd #81

Closed SaadArdati closed 5 years ago

SaadArdati commented 5 years ago

The plugin is missing basic touch responses like a regular tap. These tasks should all be passed from GestureDetector directly instead of the plugin handling its own type of gesture handling. In it's current state, it is not following any standards and is certainly not api friendly. A lot of the touch input types even trigger more than once (several times in fact).

imaNNeo commented 5 years ago

Hi buddy, Touches handled by the chart because the standard touch is not enough for us, normal touch does not tell you that you clicked on which part of the pie chart, or it does not tell you that user long pressed on which bar of bar chart, we have to implement a custom touch system, to put useful details to the touch response, but if it does not work properly we can discuss about it. First of all, I know that It's not handled properly (also it does not have a severe performance issue), and as you know we are on early versions and I think it is ok for now (btw we should try to make it better), If you have a better idea for that, make your hand dirty by creating a pull request, and I appreciate your PRs. Thanks.

SaadArdati commented 5 years ago

Take another perspective: The way I expected it to work was that for each bar, the onTap triggers for that single bar.

Alternatively, if you want to enable the ability to manipulate all the bars from anywhere, create a controller that gets passed to the charts. That way, when you trigger the onTap of a single bar, you can do whatever you want to that bar but also can control other bars via the controller.

This is the standard to most widgets flutter uses :)

imaNNeo commented 5 years ago

You can manipulate yout bars (or any kind of element in supported charts) through the widget's constructor. The problem is that when you implement the standard onTap function it doesn't tell you in which bar clicked, and for sure, the bars inside the library is not a widget, they've been drawn by a CustomPainter. And standard touches system doesn't know about your element's that drawn by a CustomPainter.

SaadArdati commented 5 years ago

I see. I'm not going to tell you how you should write the plugin of course, but separating the bars into their own widgets might be a better idea in the long run.

Thanks for the info though.

imaNNeo commented 5 years ago

As you know use-case of the CustomPainter is exactly here, because we don't have a widget like this curved line chart, And the standard touch system doesn't know about the drawn line. image,

No problem, Thank you to.

SaadArdati commented 5 years ago

The solution would be to have a gesturedetector over the whole graph. Get the cursor's position relative to the widget via MediaQuery(iirc), then based on the x-axis of the mouse, access the y value from the graph's data.

Is that hard or out of the realm of possibility?

imaNNeo commented 5 years ago

This is exactly what we do (as you said at the end we should pass the y value from the graph's data, it means we should have our touch handling system).