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

Drag offset fix (again) #499

Closed
wants to merge 3 commits into from
Closed

Conversation

leoMehlig
Copy link
Contributor

There was a bug in the previous fix. This one should now work correctly. Sorry for that.

Leo Mehlig added 2 commits October 23, 2015 15:29
Since the if statement got a bit complex, I’ve split it up into two
separate if statements.
@danielgindi
Copy link
Collaborator

Please first match it with the logic on the Android part

Also please fix the code styling...

@danielgindi
Copy link
Collaborator

Ok logic seems fine, but curly brackets should always be on a new line for clarity.
Also there should only be one commit in such a PR...

if _dataNotSet || !_dragEnabled || 
   (self.hasNoDragOffset && self.isFullyZoomedOut && !self.isHighlightPerDragEnabled)
{
  return false
}

Sorry for the code styling (just not used to that style)…
I’ve checked the Android counterpart and though the whole gesture
functionality is quit different I found the code block that checks this
thick. The problem is that it checks the different way around…

Android code:

```java

  if (mChart.hasNoDragOffset()) {

                        if (!mChart.isFullyZoomedOut() &&
mChart.isDragEnabled())
                            mTouchMode = DRAG;
                        else {
                            if (mChart.isHighlightPerDragEnabled())
                                performHighlightDrag(event);
                        }

                    } else if (mChart.isDragEnabled()) {
                        mTouchMode = DRAG;
                    }

```
@danielgindi
Copy link
Collaborator

The code that I've pasted here (which is actually the way you wrote it, only styled) matches the android logic, I checked. Android and iOS thinks differently about gestures, but the logics of those flags stayed the same.

Please squash everything into one commit.

@leoMehlig
Copy link
Contributor Author

Shall I create a new PR with all the things I've changed in one commit?

@avishaan
Copy link
Contributor

Is this supposed to be fixed in v2.1.5 @danielgindi? I'm running into this issue all of a sudden and am trying to troubleshoot.

@danielgindi
Copy link
Collaborator

No its in master, a version release will take place soon.

.

@leoMehlig leoMehlig deleted the drag-offset-fix branch November 13, 2015 16:07
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.

3 participants