BEMSimpleLineGraph icon indicating copy to clipboard operation
BEMSimpleLineGraph copied to clipboard

Pan/Zoom and Variable X Axis; many other changes

Open mackworth opened this issue 8 years ago • 5 comments

Summary

Pan/Zoom and Variable X Axis; many other changes

Fixes Issues

This pull request fixes the following issues:

  • #296, #286, #282, #278, #271, #269, #261
  • Handles very large numbers of points (realistically up to thousands)
  • Restore API back to NSInteger (for Swift) and resolved internal changes
  • Fix test for white/alpha to handle extended Grey space
  • Animation without "display dots during" left dots off previously
  • Remove prev reference lines if axis turned off
  • Supports bezier curves even when line isn't being drawn
  • Top/Bottom reference lines were outside box
  • Fix bug with touchLineInputColor (doesn't change after initial setting)
  • averageLine color picked up from line if not set
  • NoDataLabel color should not default from Line (which defaults to white, same as background)
  • If label isn't used after initially being created, ensure it's removed from view
  • Avoid possible infinite loop if delegate gives a zero incrementIndex for x-axis
  • Fix one-pixel gap between y-axis and graph
  • Remove spurious NSLogs
  • If x-axis background is defaulting to colorBottom, then also use alphaBottom to match; same for y-axis and colorTop.
  • If neither X nor Y reference lines, set line's enableRefLines to NO (although default, might have been previously YES)
  • One point is handled by main routines rather than special case
  • Simplified handling of zero points
  • Many BEMNull related bugs:
    • If null data and interpolation on, then extrapolate for beginning/ending nulls.
    • Ensure all BEMNulls do not crash
    • When Bezier off and Interpolate nulls off, properly draws line segments now
    • If interpolation off, then bezier line should be interrupted by gaps (but not top/bottom).
    • If data changes to null value, ensure corresponding Dot isn't left on chart

Changes

The following changes are included in this pull request:

  • Pan/Zoom
  • Variable X Axis
  • Cubic Bezier Algorithm
  • Many others (see below for details)
  • New Features:
    • Optional user-scaling pan/zoom with double-tap to restore
    • Variable x-axis
      • Adds new delegates:
        • lineGraph:locationForPointAtIndex:
        • lineGraph:labelOnXAxisForLocation:
        • numberOfXAxisLabelsOnLineGraph:maxXValueForLineGraph:
        • minXValueForLineGraph
    • Ability to take screen shot bigger (or smaller) than screen
    • New delegate method to specify y-axis labels
    • Axis at top option
    • Cubic Bezier algorithm for smoother lines
    • Implemented numberOfXAxisLabelsOnLineGraph for indexed xAxis
    • TestBed usable on iPhone and Split View
    • NumberOfPoints entry in TestBed
    • Activity display in TestBed to test didFinish
    • Upward-compatible API

Notes

  • [x] This pull request makes breaking changes, and if so, involves the following:
    • [ ] Public API availability
    • [x] Internal functionality or behavior
      • All global vars changed to internal properties
      • All MutableArrays to Arrays
      • Use Views to group dots and labels over BEMLine.
      • Set size of major views at beginning, so all calcs based on those views
      • Moves getting data and calculations of points to new getData method
  • [x] I have run and ensured that this pull request passes all XCTests

mackworth avatar May 12 '17 16:05 mackworth

Supersedes PR #295, which also incorporates the changes mentioned above. I created a separate pull request in case the addition of the pan/zoom and variableX are determined to be inappropriate for this library. Let me know what you think, especially on API additions, and/or if you have any problems with this build, and/or if there's any thing else you want me to implement, while I'm on a roll...

mackworth avatar May 12 '17 16:05 mackworth

Thank you so much for your contributions @mackworth! As soon as we get your other PR (#295) merged in, I'll take a look at this one. If you plan on contributing more to the project, please keep on breaking your PRs as much as possible like you did.

Boris-Em avatar May 14 '17 04:05 Boris-Em

Added a few bug fixes. Also realized an ambiguity about calculations on graphDatapoints when you have pan/zoom. Does it mean calculate with the currently displayed points or all data points? I think both are useful, so I added a switch.

mackworth avatar May 29 '17 01:05 mackworth

A couple bug fixes based on field testing of my app. Noticed that the last set of commits broke the test, so undid one of my changes there.

mackworth avatar Jun 06 '17 21:06 mackworth

Hi @mackworth, is there any way you can split out a separate pull request with only the pan / zoom feature - and nothing else?

There are a lot of changes here, and it is very difficult to accurately sift through everything. I do, however, have a few thoughts on what I have seen:

  • Zoom and pan feature seems really nice in the brief tests I did.
  • Not sure how I feel about the cubic curve change from quadratic curves. Could be a matter of personal taste, but I don't like how this looks as much. While some regions of the graph with shallow slopes are displayed better, there's an odd kind of "tilt" to each large curve.
  • Some of these added and updated APIs would be much appreciated (in fact they would address many of the issues currently open on this project). However, my fear is that these changes to the public API may confuse or overburden BEMSimpleLineGraphView in its current state. Before continuing to add more features I think the project needs to be refactored. This would be quite the undertaking, but I believe that if done correctly it would make pull requests likes this much easier to merge, manage, and create in the first place. @Boris-Em what do you think? Maybe we can start a refactoring project to discuss how to approach this?

Sam-Spencer avatar Jul 21 '17 21:07 Sam-Spencer