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

fix(ChartDataSet): highlight not being invoked for expected user interactions #4719 #4817

Closed

Conversation

dudenamedjune
Copy link

@dudenamedjune dudenamedjune commented Apr 23, 2022

(Fixes #4719)

Updates made in the ChartDataSet instance methods enforced by the protocol entriesForXValue and entryIndex to resolve #4719.

The method entriesForXValue used partitionIndex with a predicate that could rarely find the value, as partitionIndex uses equality checks on only one node as it halves recursively. The other change is to, a swiftui provided method for lazy rendering views, prefix, the match predicate for filtering within the partition.

There is an additional change how closest is calculated by entryIndex, possibly not needed. Previously, during the case of no entires, when a an xValue was tapped the next highest xValue was used. Example of the prior mentioned, xValue = 6.2 returns closest as 7 or even 12 whichever one was directly preceding its own position. It's now updated to return the shortest distance between x values for user touch accuracy; example: user taps x = 6.2, in an ascending array, the closest value to the left is 6 and to the right is 12; instead of returning back 12 it returns back the x value with the least difference 6.

Note: (Have no idea why but Xcode was not letting me use min with the obj-c doubles in ChartDataEntry...wasn't even recognizing it as a function 🤷‍♂️

Issue Link 🔗

ChartViewDelegate Function func chartValueSelected(_ chartView: ChartViewBase, entry: ChartDataEntry, highlight: Highlight) is not being called for every tap/touch inside the ChartView

Goals ⚽

To make charts work again

interactions(FIxes ChartsOrg#4719)

Changed the entriesForXValue and entryIndex method in the ChartDataSet
class to resolve ChartsOrg#4719. A few things were happening, entriesForXValue
was using a predicate for the partition function that would never find
the value it was looking for as it is giving you the correct location in
the tree to start your search and not actually searching; hence why the
values are rarely selected. The prefix array method for lazy rendering
views also needs a different match predicate since the partitioned array
has values that wont need to be highlighted. There is another additional
change I made in this PR related to the closest logic for entryIndex;
prior to this change it was grabbing the next index and not the closest.
Example of the incorrect closest value: xValue = 6.2 would return back
closest as 7 or even 12 whichever one was directly preceding its own
index, now it returns the shortest distance between x values so it can
be more accurate to the users touch.
Comment on lines +195 to +203
// override func chartValueSelected(_ chartView: ChartViewBase, entry: ChartDataEntry, highlight: Highlight) {
// super.chartValueSelected(chartView, entry: entry, highlight: highlight)
//
// self.chartView.centerViewToAnimated(xValue: entry.x, yValue: entry.y,
// axis: self.chartView.data![highlight.dataSetIndex].axisDependency,
// duration: 1)
// //[_chartView moveViewToAnimatedWithXValue:entry.x yValue:entry.y axis:[_chartView.data getDataSetByIndex:dataSetIndex].axisDependency duration:1.0];
// //[_chartView zoomAndCenterViewAnimatedWithScaleX:1.8 scaleY:1.8 xValue:entry.x yValue:entry.y axis:[_chartView.data getDataSetByIndex:dataSetIndex].axisDependency duration:1.0];
// }
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this as it is related to demo and from the comments was in TODO land. This was causing the demo app to break when selecting a value while the view was animating and then swiping back to the list screen before it was finished.

@dudenamedjune
Copy link
Author

For more context I had a look at most of the PR's that were open for the highlight fix, and the ones I looked at were not solving the root cause (predicate for the partition method) an only Highlight some of the time. With that said I think we should look into being a bit more specific with the gestures that are triggering highlight as every touch blast through most of the code in ChartDataSet class.

@dudenamedjune
Copy link
Author

@danielgindi @pmairoldi @liuxuan30

@dudenamedjune dudenamedjune changed the title fix(ChartDataSet): highlight not being invoked for expected user fix(ChartDataSet): highlight not being invoked for expected user interactions Apr 23, 2022
@dudenamedjune dudenamedjune changed the title fix(ChartDataSet): highlight not being invoked for expected user interactions fix(ChartDataSet): highlight not being invoked for expected user interactions #4719 Apr 25, 2022
@giantramen
Copy link

Tested this against a pretty wide group of CombinedCharts and was seeing the selection issue fixed.
@danielgindi @pmairoldi @liuxuan30 this bug is really bad and this fix should be merged ASAP.

@pmairoldi
Copy link
Collaborator

Closed in favour of #4721

@pmairoldi pmairoldi closed this May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants