Boris-Em / BEMSimpleLineGraph

Elegant Line Graphs for iOS. (Charting library)
MIT License
2.65k stars 381 forks source link

Y-Axis label, possible? #30

Closed japanconman closed 10 years ago

japanconman commented 10 years ago

Ever think of adding Y-Axis label?

Boris-Em commented 10 years ago

Hi @japanconman, This is a feature that I would like to see implemented as soon as possible. Unfortunately, I'm pretty busy right now and won't have time to work on it in the near future. Would you be interested on working on this feature?

japanconman commented 10 years ago

@Boris-Em : yeah, i would like to help. Just not sure I'm up to your coding standard :D. Kinda new to GitHub, how can I commit changes to the "feature" branch?

Boris-Em commented 10 years ago

@japanconman, Great news! You won't be able to commit changes directly to the project. What you can do though, is fork the feature branch of the project, work on the new feature and then open a pull request on the same branch, which I will be able to review and merge to the project (with a new commit).

I think that the best way to design this new feature, would be to make it as close as possible from the way the X-Axis Labels work. Keep in mind that it should stay optional.

Let me know if you want to talk about it more, or if you need help with anything!

Thanks a lot for the help!

japanconman commented 10 years ago

Hi @Boris-Em , there are few things I would like to add in as well

. See what's your opinion.

  1. Multi-line text support for X-Axis label.

    I've added it in, but I didn't touch too much on the constant that you are using i.e. "padding", "labelXaxisOffset.
    screen shot 2014-06-03 at 5 29 41 pm

  2. Y-Axis label with aware of autoScaleYAxis

    I'm thinking when autoScaleYAxis=YES, it's cleaner to just show the min, median and max value, whereas absolute range is used for autoScaleYAxis=NO

  3. Overlapping label removal

    The intent is to remove overlapping labels on both axis when there are just too many values. However, this contradicts with numberOfGapsBetweenLabelsOnLineGraph method. What you think?

  4. Subtle reference line for axis value across graph

    See the red arrows. The reference line will appear according to the axis label. Looking at the code structure, I'm not sure where's the best place to put this in. (Need some help on CG drawing also) stocks

Boris-Em commented 10 years ago

Hi @japanconman, Sorry for the late reply. I'll take a look at the pull request you submitted this week end. From what I saw so far, it looks amazing!

Thank you so much for all the hard work! This is truly great!

japanconman commented 10 years ago

@Boris-Em , sounds good.

On pt 4, I'm wondering what if we implement it right in the BEMLine class. Since all the data point references and CG drawing are done within that class.

jasonmcdermott commented 10 years ago

+1 for pt3, that would be super useful +1 for pt 4, though again it's good for this to be optional on both X and Y axes.

This lib is kicking some serious ass! Two months ago I was disappointed by the lack of featured, beautiful graphing libs available on iOS. I was even considering using webKit + JS! Not anymore, BEMSimpleLineGraph has it all covered, with more features on the way. Awesome.

Boris-Em commented 10 years ago

@japanconman Thanks a lot for your PR. It looks great!

@jasonmcdermott Thanks for all your help with the library. Your inputs are spot on!

Sam-Spencer commented 10 years ago

@japanconman @Boris-Em @jasonmcdermott The requested feature, (point four) is now implemented on the feature branch. See commit 2b16d643bf4e5f7db5a7d25cc117901531624a1a.

I should note however that there is still work to do on this feature. As of now it only supports x-axis / vertical reference lines and an x-axis label frame. I have not yet added support for the y-axis or multiline x-axis label (these features seem to be incomplete and little bit wonky).

Additionally, in lieu of the git-spirit I would suggest making a separate issue for each of those points. Then, as we work on them we can "cross them off" and manage them individually.

japanconman commented 10 years ago

@sam-spencer, it just happened that I tried put in the Y reference line yesterday too. Based on the committed codes, our codes are very similar. :) (just by l

japanconman commented 10 years ago

(Fat fingers typing on the phone...)

...(just by looking at codes only, haven't test on Xcode)

For Y reference drawing what I did was separate out the drawLine method from drawDots. And call the drawLine after drawYAxis. Other than that, it's pretty close.

I notice that drawDots have to be called before drawYAxis, so there are available yAxisValue for drawYAxis to create the labels.

I guess that should be the key. Will input more when I got chance to sit in front of Xcode these few days.

Cheers.

Boris-Em commented 10 years ago

As advised by @Sam-Spencer, I opened a new issue (#40) for point4, and another one (#41) for the Y-axis label support. Please continue the discussion regarding these specific features on the appropriate issue. Thank you! Closing this issue as duplicate.