Skip to content
This repository has been archived by the owner on Dec 2, 2020. It is now read-only.

Added a bunch of options including left/right y axis positioning, float values in Y-Axis and popovers, and null graph values! #129

Closed
wants to merge 10 commits into from

Conversation

RobDay
Copy link
Contributor

@RobDay RobDay commented Feb 20, 2015

Hi,

First, thanks a lot for the library. We've had some fun implementing some charts with it and adding some features to match our designs. To summarize the features that were added:

  • New property added to swap Y-Axis position from the left or right of the graph
  • Reference lines now support dash patterns as an alternate style
  • Dots can be disabled when animating the line
  • Moved the pan gesture recognizer to standard touches began .... touched ended. This allows the popover to show on the initial click rather than needing to drag around first.
  • Null values can now be returned for valueForPoint. A new variable, BEMNullGraphValue, was defined to represent a null value (CGFLOAT_MAX)
    • When drawing null values, they can either be skipped leaving a gap in the line or they can be interpolated by drawing a line through non-null neighboring values.
  • Added properties that allow the customization of the no data label's font and color
  • Added new datasource methods to support a custom popover view. The datasource is able to define a custom view and then modify the view whenever the user touches and drags on the graph
  • Added support for float values in the Y-Axis as well as the stock popover. These floats are formatted with a custom formatting property (formatStringForValues). Additionally, changed how the required size is calculated for the stock popover and the Y-Axis. Because they now support floats, the largest value is not necessarily the widest value.
  • Added new datasource methods to define a prefix and/or suffix for the Y-Axis and popover values

We hope you like the changes. We have some more changes in the pipeline. Within the next few weeks, we'll be adding support for multiple lines to be drawn on one graph. Additionally, we'll be adding some new delegate / datasoure methods for more customization around the values being chosen for the X-axis and Y-axis.

Rob Day and others added 3 commits February 20, 2015 09:33
  Y Axis Position on right
  option to dot reference lines
  option to disable animation of dots
  Float values on the Y-Axis (Controlled by 'N' sized characters)
  Customizations to the no data label (Modify the color and font)
  Optional prefix and suffix for both the popup label as well as the Y-Axis
  Option to pass a custom popover view to generate a very custom popover
…nizer for detecting touches and movement. This allows popovers to initially show without needing to drag.
…lls, an option was added to interpolate the value (to draw a line through the average of the surrounding values).
@Sam-Spencer
Copy link
Collaborator

Thank-you for all the hard work!! I'm glad that you've found it useful and are working on improving it. 🎉

That said, this is quite a large pull request! It will take some time to review all of the components of the request. Honestly, this is fantastic and it looks solid. We just need time to ensure that it's stable enough to be merged. Additionally, it may be helpful if you highlight key new features in the README.md and point out any breaking changes in the PR description.

@RobDay
Copy link
Contributor Author

RobDay commented Feb 23, 2015

Sounds good. There were no breaking changes so we should be good on that front. I can modify the readme or wiki (whichever is preferred) with the additional functionality.

Let me know if you have any questions about what we did, why we did it, etc!

@bryandunbar
Copy link

@RobDay do you have an update to the sample project to show some of the new features?

@RobDay
Copy link
Contributor Author

RobDay commented Feb 23, 2015

@bryandunbar No, but I can add them. I considered adding them to the sample project but it would make the UI pretty cluttered if I added some of the new options. Would it be sufficient to modify some of the implementation files by explicitly setting some of the new values and comment accordingly? If the sample project had an options pane, it would make adding features like this easier for the future.

@RobDay
Copy link
Contributor Author

RobDay commented Feb 23, 2015

It seems that most of the heavy documentation lives in the wiki as opposed to the readme. If you have a preferred way for me to provide wiki changes, let me know. If now, shall I fork the wiki, make my necessary changes, and point you to my fork with a commit of the respective changes?

@bryandunbar
Copy link

Yeah I agree it may be worth just adding a table view listing of different types of samples. But for ease maybe just add some commented out examples that people could swap in if they wanted to see.

I have made a good bit of changes to a fork of the project as well that I was going to submit a pull request for. But I'm considering using yours instead. The only thing I have that I don't think you are handing are purely custom labels on the axis and the points using delegates. For example, I have a chart whose data points show a time like 14:13 for 14 minutes and 13 seconds. The data for the point is simple the number of seconds and I have a delegate call back to format the output value.

On Feb 23, 2015, at 8:55 AM, RobDay notifications@github.com wrote:

@bryandunbar No, but I can add them. I considered adding them to the sample project but it would make the UI pretty cluttered if I added some of the new options. Would it be sufficient to modify some of the implementation files by explicitly setting some of the new values and comment accordingly? If the sample project had an options pane, it would make adding features like this easier for the future.


Reply to this email directly or view it on GitHub.

@RobDay
Copy link
Contributor Author

RobDay commented Feb 23, 2015

I'll add the comments to the sample app and give you an additional request this week.

In terms of the missing features, that's actually on our plate for either this week or next. We have two big changes pending:

  • Support multiple lines on one graph
  • "Smarter" axis labels.

Multiple lines is pretty self-explanatory. Smarter axis labels seems to be something that you've tackled. With our current code, our X-Axis has times like 11:14 AM. Ideally, these would be pretty round numbers. Similarly, our Y-Axis (which now supports floats!!!) has numbers like 53.37.

The planned change would be to add the ability to specify precisely what the axis labels would show. I'm not sure of our exact implementation strategy, but it is worked that we had planned for our current sprint. Does your change support fully custom labels? Our goal would be for some magic delegate call to ask a question, you pass back either an array of label values (or perhaps a base label value and an increment), and the library draws the labels in the proper location based on the graph's coordinate system.

Matthew Gambill and others added 7 commits February 24, 2015 10:39
…re recognizer for detecting touches and movement. This allows popovers to initially show without needing to drag."

This reverts commit 6daaa32.
…place the X and Y Axis values. These methods:

- (NSInteger)baseIndexForXAxisOnLineGraph:(BEMSimpleLineGraphView *)graph;
- (NSInteger)incrementIndexForXAxisOnLineGraph:(BEMSimpleLineGraphView *)graph;
- (NSArray *)incrementPositionsForXAxisOnLineGraph:(BEMSimpleLineGraphView *)graph;

- (CGFloat)baseValueForYAxisOnLineGraph:(BEMSimpleLineGraphView *)graph;
- (CGFloat)incrementValueForYAxisOnLineGraph:(BEMSimpleLineGraphView *)graph;
… framework supports. Some of these implementions are in comments as to not chance the behavior of the sample project too much.
@RobDay
Copy link
Contributor Author

RobDay commented Feb 27, 2015

I have a new pull request ready to be created. The latest code lives at: https://github.com/dowjones/BEMSimpleLineGraph. This pull request would build on top of this request adding:

  • Changes to how the pan gesture recognizer works to allow the first value to show with a tap instead of a drag
  • An optimization for nil handling with bezier paths when multiple nulls were present
  • Customizations to the values presented on both the X and Y Axis.
  • Changes to the sample project to add examples on many of the changes (Some are in commented out code blocks as to not change the sample project too much)

Being that all of these changes are built on top of this request, should I give you a second request that comprises everything in this request PLUS the new changes? Or would you rather it a different way?

@Boris-Em
Copy link
Owner

Thank you so much @RobDay for this very solid PR. I will try to find time in the next comming days to review it and hopefully integrate it to the project.
I would rather have you create a new PR with all of the changes if you don't mind. When you do so, you can close this one.
As far as documentation goes, we would like to keep the README file as simple as possible. I think that documenting all of these changes to the wiki is what makes the most sense.
Once again thank you for the work!

@RobDay
Copy link
Contributor Author

RobDay commented Feb 27, 2015

Roger, will close this and add the second.

The new pull request is located here: #132

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

Successfully merging this pull request may close these issues.

None yet

5 participants