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

General discussion & news #92

Closed
danielgindi opened this issue May 19, 2015 · 17 comments
Closed

General discussion & news #92

danielgindi opened this issue May 19, 2015 · 17 comments

Comments

@danielgindi
Copy link
Collaborator

Hi everybody!

Instead of announcing stuff through README updates, or speaking to you through personal email, I want to have this here as a place for communications.

In general, you should know that everything is done on both iOS and Android platforms so sometimes it requires Phil and I thinking about stuff together. Phil is wonderful and very cooperative so I do not feel like the communication adds any overhead. Of course sometimes we are working on different features at the same time and it takes time to review, merge etc. (And btw, we also have personal lives! I mean I don't know about Phil, but I certainly do :-) )


Just so you all know, I've had a few busy days and although I did pay some attention I did not work on any major features. In the near few days I hope to get back to implementing important features or solving bugs.

As usual, anyone wanting to participate by helping with bugs or new features - is most welcome.
If you don't want to or don't feel that you are there yet, it's also ok to buy me a beer ;-)

@AlBirdie
Copy link
Contributor

I've got two more additions to the library that I'd like to contribute.
The first thing is basically just a small adjustment to what's already there;
You can configure the xAxis to be enabled or not. If false, neither labels, nor grid lines will be rendered.
If true, you'll get both labels and the grid. That's fine. Now you can also set 'drawLabelsEnabled' on the xAxis. If true you'll see the xAxis labels. If false, you don't get labels, but the offset of the chart doesn't adjust.
In order to fix this (and therefore not waste any space by assigning space to invisible xAxis labels), a tiny adjustment has to be made;
In 'BarLineChartViewBase.calculateOffsets()' change 'if(xAxis.isEnabled)' to if(xAxis.isEnabled && xAxis.isDrawLabelsEnabled)'. That'll cause the chart area to stretch to the available height.
The same applies for the Android implementation.

The second addition is a getter and setter for the y axis width. That way you can easily adjust the axis width to a given value if you've got multiple charts in a single view. Simple use case, a finance chart with a main chart showing the instrument and several technical studies in separate charts below the main chart. Naturally they all share the same x values and zooming / panning is synched across all charts. In order to have the x grid at the same position all the time, you need the y axis to be the same width for all charts. During initialisation of the charts, just look for the widest y axis and adjust all other chart axes accordingly.
I can make a pull request for that if you like.

Third addition would be a right text alignment for the right y axis. That basically just requires a new property on the y axis and inside 'ChartYAxisRenderer.renderAxisLabels' something around the lines of this;

if (labelPosition == .OutsideChart)
{
    if(labelAlignment == .Left)
    {
        textAlign = .Left;
        xPos = viewPortHandler.contentRight + xoffset;
    }
    else
    {
        textAlign = .Right;
        xPos = viewPortHandler.chartWidth; // we could add the offset here as well so that the labels a positioned a little to the right
    }
}

Last (but probably not least ;)) thing is a 'drawLastXValue' property that draws a little box containing the value of the last visible x value on top of the y axis. This is very common in finance charts, I just don't know how much sense this would make in case the y axis is set to the left. What do you guys think?

@danielgindi
Copy link
Collaborator Author

@AlBirdie I'm with you - any new property to customize the visibility of chart, which doesn't hurt current code - is a welcome feature which gets merged almost automatically ;-)

@danielgindi
Copy link
Collaborator Author

We are reaching 3k stars in just two months :-)

@AlBirdie
Copy link
Contributor

Way more to come! :)

I've got another 'issue' with the current implementation that I'd like to discuss. All extras a renderer can draw can be toggled on the underlying DataSet, such as values (isDrawValuesEnabled), circles (isDrawCirclesEnabled), and so on. Unfortunately there is no such thing for the highlights. They will always be drawn for all sets. My proposal would be another boolean flag on the set (isDrawHighlightEnabled), so that the user can specify which set in a CombinedChart to highlight.

@AlBirdie
Copy link
Contributor

Daniel, do you want a pull request for the bespoken "drawLastXValue" box as well (screenshot attached)?

bildschirmfoto 2015-05-26 um 18 50 41

@danielgindi
Copy link
Collaborator Author

How configurable is it?

@AlBirdie
Copy link
Contributor

Well, obviously you can set the colors of the control (text and background) and the font itself.
It is configurable for each dataset, so all settings as well as enabling/disabling the control takes place on the corresponding dataset.

@danielgindi
Copy link
Collaborator Author

Sounds great :-)

@AlBirdie
Copy link
Contributor

Come to think of it, since I'm using a right axis exclusively, I haven't made this configurable for the left axis yet. I feel that if you guys were to include it into the library, this control would have to work for all scenarios (left only, right only, left and right), wouldn't it? If you agree to that, I'd implement that in the next couple of days (probably next week).

@danielgindi
Copy link
Collaborator Author

Yes that's right :)

‏בתאריך יום רביעי, 27 במאי 2015, AlBirdie [email protected] כתב:

Come to think of it, since I'm using a right axis exclusively, I haven't
made this configurable for the left axis yet. I feel that if you guys were
to include it into the library, this control would have to work for all
scenarios (left only, right only, left and right), wouldn't it? If you
agree to that, I'd implement that in the next couple of days (probably next
week).


Reply to this email directly or view it on GitHub
#92 (comment)
.

@AlBirdie
Copy link
Contributor

Alright, done. Waiting for the other pull request to go through first, though.
Next one will be the proper alignment of the y axis labels (left, center, right).

I think that's it for the time being from my side then. There are some other minor changes and additions such as an OHLC chart and a stepped line chart, but I don't see the need for a pull request for those. OHLC is already part of the Android library and my implementation probably differs a bit, and the stepped line chart is probably not required to be included here.

@danielgindi
Copy link
Collaborator Author

I want to try something - animating between arrays of YVals...
That would add a nice-to-have feature that's already in HighCharts.

To accomplish that we would need to:

  1. Assume that the yVals before and after are on the same xIndexes
  2. Update the yVals array
  3. Keep 3 extra nullable arrays, for: source yVals, dest yVals, and current - with only the values that are currently in the viewport.

Any thoughts? @AlBirdie do you want to try to implement this? ;-)

@AlBirdie
Copy link
Contributor

AlBirdie commented Jun 3, 2015

Argh... totally forgot to implement the 'drawStartEndXValue' on the demo project and make a pull request for it. That'll have to come first. :)

Regarding your yVals animation, I didn't really get what you mean to be honest. Do you have a working sample for this, maybe from the demos available at HighCharts/HighStocks?

@danielgindi
Copy link
Collaborator Author

danielgindi commented Jun 3, 2015 via email

@gjpc
Copy link

gjpc commented Jun 4, 2015

Great collaboration Daniel, your ios-charts contribution is much appreciated. Your instructions for integrating the package is a bit terse, especially for first time ObjectiveC++/Swift integrators.

I decided to include IOS Charts in my project but did not want to mingle the sources in the same directory, so I placed the Charts Project folder next to my code's project folder. I dragged the Charts project into my Project's Navigator Bar and included the framework in the my project target's Embedded Binaries list in the General project settings and set the Embedded Content Contains Swift Code switch to yes in my project's Build Settings tab in the Build Options section.

My project's moduleName-Swift.h file would never generate no matter what other switches I tried. Finally seeking out the -Swift.h files with a find on my project's xcode Build directory, I saw that a Charts-Swift.h file was being generated deep in my project's xcode Build directory in Charts.framework/Headers/

The solution to using your ios-charts Swift package without including the code in your own project's source directory is to use this import:

#import "Charts/Charts-Swift.h"

Thanks again for a great package.

P.S. Setting the ChartsDemo project to Universal and allowing all rotations gives a much more satisfying demo on the iPad.

@gjpc
Copy link

gjpc commented Jun 8, 2015

I am continuing to plug along with my bio data app enjoying learning ios-charts.

I did something really cheesy this morning adding a valueIsIndex Boolean to the ScatterChartDataset class and the data point switcheroo code in ScattterChartRenderer class.

I think the package would benefit with a value formatter that that takes the index of the dataset and returns the value text string, perhaps even using adjacent points to derive the value. Perhaps call it complexValueFormatter?

Worth my time to code and debug?

@liuxuan30
Copy link
Member

@danielgindi Not sure if you have seen this:
http://www.appcoda.com/ios-charts-api-tutorial/
Quite a good article to get new comers started

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

No branches or pull requests

4 participants