Feature - ChartView Pan Ended Delegate Call

[AntonTheDev]

Issue Link 🔗

Over the years, every I have come to implement this library, i have found that 1 feature for me has always been missing which is the ability to know when the user stopped panning across a chart to display values on the fly, and have it reset once the user lets go. I've yet to find a clean solution other than modifying the library, and I have added some callback logic to the ChartViewDelegate

Goals ⚽

To be able to install this from the master repo, and not always have to fork it to add this exact feature./

Implementation Details 🚧

Added and optional method to the ChartViewDelegate to tell the delegate that the pan gesture has reached a cancelled / ended state

public protocol ChartViewDelegate
{
     ...

    /// Called when a user stop highlighting values while panning
    @objc optional func chartViewDidEndPanning(_ chartView: ChartViewBase)
}

Testing Details 🔍

None, there are no specific tests. This should not effect the snapshotting at all, and has no effect on the existing codebase other than an optional callback.

[codecov-io]

Codecov Report

Merging #3612 into master will increase coverage by 1.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3612      +/-   ##
==========================================
+ Coverage   29.94%   31.01%   +1.06%     
==========================================
  Files         118      114       -4     
  Lines       13793    10483    -3310     
==========================================
- Hits         4130     3251     -879     
+ Misses       9663     7232    -2431

Impacted Files	Coverage Δ
Source/Charts/Charts/BarLineChartViewBase.swift	65.78% <ø> (+39.25%)	⬆️
Source/Charts/Charts/ChartViewBase.swift	19.51% <ø> (-2.08%)	⬇️
Source/Charts/Utils/Platform.swift	0% <0%> (-15.39%)	⬇️
Source/Charts/Charts/PieRadarChartViewBase.swift	5.63% <0%> (-14.07%)	⬇️
Source/Charts/Charts/CombinedChartView.swift	25% <0%> (-13.62%)	⬇️
...s/Data/Implementations/Standard/PieChartData.swift	18.91% <0%> (-13.29%)	⬇️
...s/Data/Implementations/Standard/ChartDataSet.swift	23.33% <0%> (-12.94%)	⬇️
Source/Charts/Utils/ChartUtils.swift	36.87% <0%> (-9.24%)	⬇️
Source/Charts/Utils/ViewPortHandler.swift	18.13% <0%> (-8.72%)	⬇️
Source/Charts/Utils/Platform+Accessibility.swift	31.57% <0%> (-5.46%)	⬇️
... and 63 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4fca1f7...33e8eb1. Read the comment docs.

[jjatie]

@liuxuan30 @petester42 This looks good to me!

[jjatie]

I believe your documentation needs to be updated. Shouldn't this read something like Called when a user stops panning?

[liuxuan30]

documentations need to be precise.

If many people want the delegate, we could merge.

[proped]

I need this. Could u merged it ?

[AntonTheDev]

I updated the comment accordingly in the code, hope this helps :)

[natan]

@liuxuan30 Any update on this?

[pmairoldi]

These changes seem fine to me. If we’re going add delegate calls for gesture recognizes we should think about if we need them for all states of the gestures. Do people care about when a pan starts, or when it is ongoing?

[liuxuan30]

I'm open to merge.but as @petester42 said, if we add this, maybe we should add more?

[natan]

@liuxuan30 @AntonTheDev Great, thank you for moving this along!

[liuxuan30]

shall we add at least didStartPanning to pair for this PR?

[AntonTheDev]

I believe the whole "didStartPanning" can be be assumed when "chartValueSelected" is called? Since you can't really start panning without making a callback. But once you take your finger off the screen, there was no way to tell that you did. The values do no unselect themselves

[anton-filimonov]

I believe it'd be useful to make the callbacks similar to what we have for scrollview. And actually call them in pretty much the same situations as ScrollView does.

[jjatie]

I second making the delegate behave and be named as closely to UIScrollViewDelegate as possible.

[liuxuan30]

@AntonTheDev I'm ok to merge this, but it's just strange to me that we had didEnd without didStart. Though you might be able to know it get started through chartValueSelected, I think it's still different events.

How about we append the didStartPanning in recognizer.state == NSUIGestureRecognizerState.began closure?

[Arkezis]

Thanks for your awesome library ! It is really helping us :)
This PR would be awesome for our application, we are looking forward to merge it :)

[liuxuan30]

@petester42 @jjatie how about this PR? shall we add a few more or just merge? I'm looking for a few PRs that can be merged soon and release a new version next week.

[jlubeck]

@AntonTheDev I'm ok to merge this, but it's just strange to me that we had didEnd without didStart. Though you might be able to know it get started through chartValueSelected, I think it's still different events.

How about we append the didStartPanning in recognizer.state == NSUIGestureRecognizerState.began closure?

Yes please! I'm running into this situation myself right now. chartValueSelected get called both on drag AND tap, but chartViewDidEndPanning only gets called when the user stops dragging. Definitely different use cases...

Specifically, when this chart gets embedded inside a UIScrollView, we want to stop the UIScrollView from dragging when we are dragging on the chart. So one would think we can stop the UIScrollView from dragging on chartValueSelected, and restart it on chartViewDidEndPanning

But the problem with this solution is that chartValueSelected is also called on a single tap on the chart. So when this happens, the UIScrollView stops dragging, but then we have no callback to restart it... And the user is stuck with an unscrollable UIScrollView until get actually drags on the Chart.

Hope this was clear. I'm about to leave for the day, but if I don't hear from anyone back I'll probably make a pull request myself tomorrow.