Consolidate react-native MPChart libraries

[mikemonteith]

[Taakn]

Hi @mikemonteith, I was wondering if you were still working on merging your library with @Jpadilla1. Thanks!

[mikemonteith]

@Taakn That's the plan. Although getting the Android version as complete as possible is the priority for me at the moment. (pull requests welcome for anything ios based)

[Jpadilla1]

@Taakn I'm on the same path as @mikemonteith to get the iOS version as complete as possible. Along the way, we could merge both libraries when we are happy enough with the completion.

[Taakn]

Ok thanks so much for your work on porting MPChart to React Native!

[Taakn]

Ok so thanks to you both, I managed to get your chart libraries working on React 0.19.0 on both iOS and Android.

I have a wrapper that works as follows, so I don't think it really matters to have one repo for iOS, and one repo for Android:

if (Platform.OS === 'ios') {
  var { BarChart } = require('react-native-ios-charts');
}

if (Platform.OS === 'android') {
 var BarChart = require('react-native-mpchart').BarChart;
}

module.exports = {
  BarChart: BarChart
};

It gets a little more complicated when I want to get a chart working on both, for instance:

<BarChart
          style={{flex: 1}}
          ///////////////////////////////////////////////
          // android
          ///////////////////////////////////////////////
          data={{ // android: data / ios: config
            dataSets:[{
              values: [1,2,3,4,5,6,7],
              colors: ['#990000'],
              drawValues: false,
            },{
              values: [4,3,4,2,1,3,1],
              colors: ['#009900'],
              drawValues: false,
            }],
            xValues: ["A","B","C","D","E","F","E"],// android
            highlightEnabled: false,
          }}
          gridBackgroundColor="#33990000" // android (transparent red)
          backgroundColor="white" // is (transparent red, alpha does not work)
          touchEnabled={false} // android only
          // Axis for Android
          leftAxis={{
            //minValue: -12,
            //maxValue: 12,
            drawGridLines: false,
            //inverted: true,
          }}
          rightAxis={{
            enabled: false,
          }}
          xAxis={{
            enabled: false,
          }}
          ///////////////////////////////////////////////
          // ios
          ///////////////////////////////////////////////
          config={{ // android: data / ios: config
            dataSets:[{
              values: [1,2,3,4,5,6,7],
              colors: ['#990000'],
              drawValues: false,
            },{
              values: [4,3,4,2,1,3,1],
              colors: ['#009900'],
              drawValues: false,
            }],
            labels: ["A","B","C","D","E","F","E"],// ios
            highlightEnabled: false,
            xAxis: {enabled: false},
            leftAxis: {drawGridLines: false},
            rightAxis: {enabled: false},
            pinchZoomEnabled: {false},
            doubleTapToZoomEnabled: {false},
            highlightPerTap: {false},
            gridBackgroundColor: '990000', // android (transparent red)
            showLegend: {false},
          }}
        />

It's almost the same basically, with the exception that @Jpadilla1 you wrap everything inside a config attribute, and @mikemonteith you use a data attribute to wrap around your datasets.

So I don't know how you want to proceed in terms of interoperability. For me it's not a big deal given that I can write a wrapper for both, but it would be fantastic to have two components that are as similar as possible when you want to create a chart.

Anyway I think you're both doing a fantastic job and I really want to thank you for everything that you're doing, but also for answering my questions when I was trying to get your libraries up and running.

[Jpadilla1]

@Taakn you are very welcome and that's awesome that you got both of our libraries working!

The reason why I only use 1 property for everything is that in my experience with iOS and the iOS-Charts library, every time I passed a new property it triggered a render. So let's say we have a BarChart component like this

<BarChart data={...} pinchZoomEnabled={false} gridBackgrounColor='99000'/>

That would trigger 3 renders. So to avoid that I use just 1 property config and it only triggers 1 render. @mikemonteith does this happen to you in your library?

If their is a better way I'm open ears :)

[Taakn]

@Jpadilla1 It doesn't sound like that would be standard React component behavior. Maybe because you're using native components? Were you able to measure a difference in performance?

[mikemonteith]

@Jpadilla1 is the 3x re-render happening in the React layer or native? I think the native layer should be able to handle props changing so as to only re-render when needed but code will need to be written to handle that. React should handle changing props in it's own way and make it's own performance optimisations.

[Jpadilla1]

Hey @mikemonteith I'll re visit this issue and get back to this thread.

Let's decide on how the properties are going to be passed over to iOS/Android. I think that since the libraries have a lot of properties that it's best to have a single configuration object with all the properties. That would make the JSX much cleaner and properties can be shared across multiple charts.

Also, I want to finish supporting all the charts of ios-charts (only missing CombinedChart) and then I would like to focus on Android. Do you need help?