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 for issue where data points are not highlighted #4610

Closed
wants to merge 1 commit into from

Conversation

gavynriebau
Copy link

Issue Link πŸ”—

Fixes #4609 and possibly also #4579

Goals ⚽

Improve the highlighting / selection behavior of line charts

Implementation Details 🚧

The entriesForXValue func finds the data entries for a given x-axis
value. It used an optimization where it would do a binary search to find
the first matching entry then take a slice of data stopping when no more
entries matched.

The optimization broke the highlight feature because data points that
should have matched were not matching resulting in no entries being
returned.

This commit replaces the optimized code with a call to .filter which
will be slower for larger data sets but correctly returns filtered
entries.

Testing Details πŸ”

Tested by altering the demo project to use the forked version of the code and confirming the broken behaviour works after the fix is applied

The `entriesForXValue` func finds the data entries for a given x-axis
value. It used an optimization where it would do a binary search to find
the first matching entry then take a slice of data stopping when no more
entries matched.

The optimization broke the highlight feature because data points that
should have matched were not matching resulting in no entries being
returned.

This commit replaces the optimized code with a call to `.filter` which
will be slower for larger data sets but correctly returns filtered
entries.
Copy link

@mlatham mlatham left a comment

Choose a reason for hiding this comment

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

I also encountered this issue, so bump on this PR. I took a different approach to fixing it in our fork that I think addresses the regression while maintaining the optimized approach. The partitioningIndex just wasn't being calculated correctly.

/// - Returns: All Entry objects found at the given xIndex with binary search.
/// An empty array if no Entry object at that index.
open override func entriesForXValue(_ xValue: Double) -> [ChartDataEntry]
{
    let match: (ChartDataEntry) -> Bool = { $0.x == xValue }
    let i = partitioningIndex { $0.x >= xValue }
    guard i < endIndex else { return [] }
    return self[i...].prefix(while: match)
}

@gavynriebau
Copy link
Author

@mlatham thanks for the code example, I tested out your fix and think you're correct it was just the filter predicate that was wrong.

In light of the above I'm closing this PR and have opened a simpler PR which still fixes the issue here:
#4613

@HWS123
Copy link

HWS123 commented Sep 10, 2021

nice

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

Successfully merging this pull request may close these issues.

Highlighting buggy / not working consistently
3 participants