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

Bug in ZoomViewJob? #3299

Closed
wuxudong opened this issue Feb 26, 2018 · 3 comments · Fixed by #3312
Closed

Bug in ZoomViewJob? #3299

wuxudong opened this issue Feb 26, 2018 · 3 comments · Fixed by #3312
Assignees
Labels

Comments

@wuxudong
Copy link

In an react native wrapper project , It use Charts as underlying lib.

After updating Charts from 3.0.3 to 3.0.5, the zoom is not behaving as expected.

barLineChart.zoom(scaleX: CGFloat(json["scaleX"].floatValue),
                    scaleY: CGFloat(json["scaleY"].floatValue),
                    xValue: json["xValue"].doubleValue,
                    yValue: json["yValue"].doubleValue,
                    axis: axisDependency)

In Charts 3.0.5, it doesn't center at (xValue, yValue), but in Charts 3.0.3, it works well.

After dig a little further, I found in commit #3041 , the code in ZoomViewJob changed from

var pt = CGPoint(
    x: CGFloat(xValue - xValsInView / 2.0),
    y: CGFloat(yValue + yValsInView / 2.0)
)

to

 var pt = CGPoint(
	x: CGFloat(xValue - xValsInView) / 2.0,
	y: CGFloat(yValue + yValsInView) / 2.0
)

notice the position of brackets, the computed pt is totally different.

is it an unintentional wrong typing ? please fix it.

@liuxuan30
Copy link
Member

liuxuan30 commented Feb 27, 2018

will this cause that big difference? CGFloat(xValue - xValsInView / 2.0) and CGFloat(xValue - xValsInView) / 2.0 are merely the same I think.

But you are welcome to test it on your side and let us know the results with proof and details.

@wuxudong
Copy link
Author

wuxudong commented Feb 27, 2018

/(division) is prior to -(subtraction).

eg. xValue = 20, xValsInView = 20

CGFloat(xValue - xValsInView / 2.0) = CGFloat (20-20/2.0) = CGFloat(10)

CGFloat(xValue - xValsInView) / 2.0 = CGFloat(20-20) / 2.0 = CGFloat(0) / 2.0 = CGFloat(0)

@liuxuan30
Copy link
Member

liuxuan30 commented Mar 1, 2018

oops, good catch. This is a mistake @jjatie . will be taken care of in 3.1

@liuxuan30 liuxuan30 added the bug label Mar 1, 2018
@liuxuan30 liuxuan30 self-assigned this Mar 2, 2018
liuxuan30 added a commit that referenced this issue Mar 2, 2018
FreddyZeng added a commit to FreddyZeng/Charts that referenced this issue Mar 11, 2018
* 'master' of https://github.com/danielgindi/Charts:
  fix ChartsOrg#3311. Need one more key for iOS 11 camera roll saving
  revert a mistake, fix ChartsOrg#3299
FreddyZeng added a commit to FreddyZeng/Charts that referenced this issue Mar 14, 2018
* master: (55 commits)
  Refactors -[tableView:cellForRowAtIndexPath:] (ChartsOrg#3326)
  fix ChartsOrg#3311. Need one more key for iOS 11 camera roll saving
  revert a mistake, fix ChartsOrg#3299
  add pie chart unit tests (ChartsOrg#3297)
  ChartsOrg#3287: align Objc and Swift demos balloon marker
  update changelog
  Min and Max reset when clearing ChartDataSet (Fixes ChartsOrg#3260)
  Restored old performance (ChartsOrg#3216)
  Support other bundle than main MarkerView.viewFromXib() (ChartsOrg#3215)
  For ChartsOrg#2840. add dataIndex parameter in `highlightValue()` calls (ChartsOrg#2852)
  Balloon Marker indicates position of data (ChartsOrg#3181)
  bump Info.plist version
  Fixed a duplicated assignment compared with obj-c code. (ChartsOrg#3179)
  Updated README for 3.0.5 (ChartsOrg#3183)
  BubbleChart uses correct colour for index now.
  Added custom text alignment for noData
  Fixes the distance issue between the legend and the horizontal bar chart (Fixes ChartsOrg#2138) (ChartsOrg#2214)
  call setNeedsDisplay() here to trigger render noDataText (ChartsOrg#3198)
  添加定制TY的Mark
  添加修改过的Mark到工程中
  ...

# Conflicts:
#	ICXCharts.podspec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants