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

Charts 3: Horizontally scrolling BarChart in a vertically scrolling UIScrollView? #1847

Open
spalmen opened this issue Nov 18, 2016 · 9 comments
Labels

Comments

@spalmen
Copy link

spalmen commented Nov 18, 2016

I'm displaying a (Charts 3.0) BarChartView in a UIScrollView (a UITableView to be specific.)

The BarChartView is scaled on the X axis only (setScaleMinima(n, scaleY:1)) so the Bar chart horizontally scrolls the data set.

What's the technique for enabling the containing UIScrollView to scroll vertically, when a vertical Pan gesture occurs on the BarChartView?

Charts.BarLineChartViewBase.panGestureRecognized(_:) purposely prevents the containing Scroll view from scrolling during the Pan gesture. Paraphrasing:

if recognizer.state == NSUIGestureRecognizerState.began {
    if self.isDragEnabled &&  // true
        (!self.hasNoDragOffset || !self.isFullyZoomedOut) {  // hasNoDragOffset: true, isFullyZoomedOut: false because viewPortHandler.isFullyZoomedOutX is false
    let translation = recognizer.translation(in: self) // .x: 0, y: -n
    let didUserDrag = (self is HorizontalBarChartView) ? translation.y != 0.0 : translation.x != 0.0
    if didUserDrag && !performPanChange(translation: translation) { // didUserDrag: false
    } else {
        _outerScrollView?.nsuiIsScrollEnabled = false  // _outerScrollView: my UIScrollView
    }

Note that my Bar chart does not scroll vertically, and the pan gesture translation is Y-axis only, yet Bar chart disables ScrollView scrolling for the lifetime of the gesture.

BarLineChartViewBase.gestureRecognizer(_:shouldRecognizeSimultaneouslyWith:) is written to allow a ScrollView superview to recognize Pan gestures, and sets _outerScrollView to my containing ScrollView, so clearly a ScrollView-contains-a-BarChartView hierarchy is supported.

I've searched all the "scroll" Issues here, and SO [ios-charts], but come up empty for a Chart 3 solution.

I'm just stumped on how to configure my scaled-on-X-axis-Bar chart to allow Y-axis pan gestures to the Scroll view.

Help appreciated. Thank you.

@liuxuan30
Copy link
Member

liuxuan30 commented Nov 18, 2016

Would you be able to provide your own gesture handler for your needs? It looks like you already had the solution. The library can't satisfy all needs, and the handler is private - you can provide you own.

@inix
Copy link

inix commented Dec 1, 2016

@spalmen
@liuxuan30

hi, I have similar scenario,that,I disable Y axis scale,but I can not scroll vertically tableview in the chart.
I just do a simple hack,that if chart disable Y axis scale,enable UITableView vertically scrolling.

		if !self.isScaleYEnabled {
			_outerScrollView?.nsuiIsScrollEnabled = true
		} else {
			_outerScrollView?.nsuiIsScrollEnabled = false
		}

in func @objc fileprivate func panGestureRecognized(_ recognizer: NSUIPanGestureRecognizer)
of BarLineChartViewBase.swift file

diff:

@@ -688,7 +688,11 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
                     if _outerScrollView !== nil
                     {
                         // Prevent the parent scroll view from scrolling
-                        _outerScrollView?.nsuiIsScrollEnabled = false
+                                               if !self.isScaleYEnabled {
+                                                       _outerScrollView?.nsuiIsScrollEnabled = true
+                                               } else {
+                                                       _outerScrollView?.nsuiIsScrollEnabled = false
+                                               }
                     }
                 }
                 

@spalmen
Copy link
Author

spalmen commented Dec 13, 2016

@liuxuan30 thank you for responding. I appreciate that Charts can't be all things to all apps.

I hope you see my point it's a poor user experience when a tap+vertical pan in one part of a TableCell scrolls the table as expected, but the same tap+pan action on the BarChartView in that cell has no effect. In my case 80% of a row's height is the chart, so most of the cell is effectively dead space.

I respectfully urge you to reconsider support for this use case. If a pan occurs on an axis in which the chart has no interest, I contend Charts should not unilaterally decide the pan gesture is of no interest to the parent UIScrollView either. IMO there should be a mechanism that allows the app code to participate in that decision. For example, if I drag diagonally across the BarChart, it may be perfectly acceptable in some UIs for the chart to scroll horizontally while the table scrolls vertically.

That aside, I perhaps misunderstand your recommendation to provide my own PanGestureRecognizer. Would you mind describing your approach in more detail?

If I add a PanGestureRecognizer to the BarChartView, I can in the associated "panGestureRecognized" action method, when recognizer.state == .changed, check recognizer.translation.y, and if it's a vertical drag, I could update TableView.contentOffset to scroll the table vertically.

But because BarLineChartViewBase.panGestureRecognized(_:) sets the TableView.isScrollEnabled false for the lifetime of the Pan gesture, the contentOffset changes made by my PGR will have no effect until the pan ends when BarLineChartViewBase reenables TableView scrolling.

My PGR delegate's gestureRecognizer(_:shouldRecognizeSimultaneouslyWith:) would be of no help here. My shouldRecognizeSimultaneouslyWith: would need to know whether it's a horizontal or vertical pan in order to decide whether BarChartView's PGR should be allowed to "RecognizeSimultaneously" the gesture. But the pan translation direction isn't known at this point.

What am I missing? Thanks!
Cheers…

@liuxuan30
Copy link
Member

liuxuan30 commented Dec 13, 2016

Hey @spalmen, thanks for writing. What I am saying is that, the gesture handlers are private func, so you can provide another gesture handler in your sub class to replace the old one, than you can do whatever you want. You shouldn't add a duplicated gesture, just replace the handlers.

I know the scrolling in a UIScrollView is a problem, there is another thread also talking about this, like #1931. It would be nice that we can have more fine-grained control over this. However, it usually is a custom situation, and you know we can't satisfy them all, and usually it might bind to business logic which is easier on your side to solve that, not us.

So bottom line: you can replace the handler to solve your problems; and you are more than welcome to file a PR for this feature, satisfying the library code style.

@liuxuan30 liuxuan30 reopened this Dec 13, 2016
@spalmen
Copy link
Author

spalmen commented Dec 15, 2016 via email

@liuxuan30
Copy link
Member

liuxuan30 commented Dec 19, 2016

I didn't feel any offend, so no apology from you is needed :)
What I wanted to say is we are short of hands and can't help for more right now about this feature.
If you want to help, you will need to walk through the library to know how the part you want to contribute works, and also refer to the code style, and just fix/implement anything you think it's helpful for others, and file a pull request!

For the gestures:
What I tried before for my project is, I subclass bar chart, remove the old gesture handler, and feed my own. So I can get full control of the gesture to some things.

If I found anything is private, I will modify the source code to make it public so I can continue. I know it's not always good, but it fix my problems. If you want to refer some private funcs without touching the source code, then you might want to really implement something based on the library, and get it merged, but it will take a long time.

So I still suggest, if needed, use the source code of this library in your project, try not to modify it unless you really do, and leave enough comments so when you upgrade the library, you will know how do you changed it before and make it easier to upgrade.

@liuxuan30
Copy link
Member

liuxuan30 commented Dec 19, 2016

I recall there is an old topic about handler's internal vs private:#268 and #148. You will find some clues there maybe

@ikyh
Copy link

ikyh commented Aug 30, 2017

@spalmen Hey I have the same issue. I have to display chart in tableView, But after graph is plotted, the tableView does not scroll vertically. I did this for now,

@objc fileprivate func panGestureRecognized(_ recognizer: NSUIPanGestureRecognizer)
{
    ............................
  else{
     if _outerScrollView !== nil
      {
        // Prevent the parent scroll view from scrolling
        _outerScrollView?.nsuiIsScrollEnabled = true
       }
    }}

but modifying the source file is wrong I guess. Also after doing this when I scroll (the tableView scrolls if above set to true) the tableView, the graph view in the cells stop scrolling and after I release the touch, they start scrolling again. So did you find any fix solution to your problem? Can you please help with this. I have mentioned the problem here https://github.com/danielgindi/Charts/issues/2757

@xavbox92
Copy link

in MyGraphTableViewCell.swift

myChartView.dragYEnabled = false

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

No branches or pull requests

5 participants