I saw that there was a new project http://drupal.org/project/ofc_api that is accomplishing the same thing as this project. It provides an interesting way of handling the charts and it seems to fix a couple of issues that exist here. I have asked the author to see if we can find a way to collaborate and post his thoughts on this thread. You can see my post to him here. http://drupal.org/node/443308

But the gist of it is, and this is up to jvandervort since he has been the one developing this lately, that if the approach given in Open Flash Chart 2 API is valid and worthy of considering we should consider making this module only support ofc v1 and push people to the other module for future development.

So why not keep the module development at this space? My only real reason is I like his module name space better ofc_api The one my rookie brain thought of at the time was way too long.

Thoughts?

Comments

kong’s picture

Actually when I wrote Open Flash Chart 2 API (ofc_api), I think of it as a "temporary solution" for people who wants to use OFC2 on Drupal 6 because I know the developers of Open Flash Chart API (open_flash_chart_api) are planning to support OFC2 in Drupal 7.

Sorry I didn't cooperate with you in the first place. Because when I saw the message on the project page I just thought "Oh, this module will not support OFC2 until Drupal 7 so I will write one for Drupal 6 and I will use their module once Drupal 7 comes out." I didn't even think about porting ofc_api to Drupal 7 :P

As for the approach I use, I just tried to follow what is in OFC2 tutorials. In the tutorial, the data file is a URL that returns JSON string; And in Drupal, a URL can be used to call a function (which can return JSON string, of course) -- to me, that just fits the concept of how to provide data to the chart.

And for the API, as OFC2 is under very active development, the best API would be the one that is being developed by the author. So I just provide a way to include an external library instead of writing my own. But there's also a built-in API which can be used to create a quick and simple charts. Or it may even be more convenient than using an external library for people who know what they are doing.

However, that was only my idea alone. So there may be a better way to do it and I'm willing to discuss with @redndahead, @jvandervort, and everyone else. If there is a better method, we can leave things like this -- ofc_api will only support Drupal 6 users and when Drupal 7 comes, open_flash_chart_api will take care of the rest (which I can also give a hand on development). Or we can create another branch of ofc_api with the new approach.

Or if we agreed that the method ofc_api used is valid and good enough, we can do what @redndahead suggested in the first place -- open_flash_chart_api will support OFC1 and future development will be based on ofc_api. Or we can also create another branch of open_flash_chart_api with the code based on what ofc_api have done so far.

So what are your thoughts on this?

jvandervort’s picture

Wow, this is all great. I'm not at all against combining forces. The reason I wrote the note about OFCv2 being unstable is because the other John (monkeyboy) is doing so much work in OFCv2 that he still regularly breaks it as he is developing new functionality. I just didn't want to get into things like this:

version 1.12.2 goes with OFC Version 2 Ichor (20th March 2009)
version 3.10.1 goes with OFC Version 2 Hyperion (6th Nov 2008)
version 3.9.12 goes with OFC Version 2 Gamera (15th Oct 2008)
And the current SVN repo head has errors in it.
(These are all good thing as they happen with active development).

Honestly every one of these broke the api. I know, I use it at work:) It's hard enough for me to follow let alone write clear enough documentation for other users.

All that being said, if you want to track OFCv2. Let's go for it.

@kong: I'd love to create a Open Flash Chart API branch -3 for you and add you to the developer list (or have redndahead do it:)

redndahead’s picture

It's fine with me if you're up to it kong. I would like to check out kongs approach because it solves the window mode issue. And it seems simpler than reimplementing the php library as I did here.

jvandervort’s picture

Yep, I'm all for using the library rather than recoding it. We'll just have to keep an eye on namespace clashes, if any.
Wanna add @kong to cvs and see if we can get him to commit to a 6.x--3 branch?

redndahead’s picture

@kong you are now added. Hopefully you're willing to do a 6.x--3 branch and commit your code to there. We (I don't know if it really can be me.) need to go through the code to make sure we understand what's going on before releasing a dev version. Then I would like to leave it at dev until ofcv2 is released. Along with a better note on the homepage that says something to the effect of "Although we feel the 6.x-dev version is stable unfortunately Open Flash Chart v2 is not. Until Open Flash Chart v2 is released expect bugs in it's usage."

Can you explain how you make sure there are no namespace clashes? My thoughts are that since the library uses classes and the class name is open_flash_chart then the only way we have a chance of clashing is if another module uses a class of open_flash_chart. If you are using 2 open_flash_chart modules then you can expect problems.

jvandervort’s picture

If OFCv2 lib uses classes we should be OK if there are no global variables that happen to match drupal.
Just something to keep looking for as the ofc lib changes.

@kong, I made sure the 6.x--3 branch was created and slapped your dev code in there. Of course if you disagree to this, I'll delete it.
I just wanted to get this started asap as I know some people are really interested in the stack bar charts.

@redndahead I agree your statement regarding the dev version. We'll have to be very explicit with calling out exact versions of OFC. John Glazebrook has started naming his releases, so we'll have to list each by name somewhere.

kong’s picture

@redndahead I didn't do any namespace check because I assume ppl would only use one module or another to create charts. But if there is a chance for namespace clashing then we may have to think of the way to work around it.

@jvandervort Wow that is fast :) One question: Is it OK to include swfobject.js in the package? Or we should ask the users to download it themselves -- because it seems to be the recommended way for embedding Flash into web page.

redndahead’s picture

We have to have them download it themselves or we need to make one of the swfobject modules a dependency. Which I like better.

jvandervort’s picture

Since swfobject.js is included in the open-flash-chart-1.9.7 and open-flash-chart-2-ichor downloads, I say we keep it in the documentation to copy it. That way we can control when it is included as well. If we rely on another module... Well, we know how hard it is to keep in sync with the correct versions.

redndahead’s picture

Ahh forgot about ofc including swf in the package. Sounds good jvandervort.

jvandervort’s picture

@kong the 6.x--3 branch appears to be working now. Can you give the May 4, nightly snapshot a quick test/review tomorrow?
I added a couple miniscule fixes so that swfobject isn't loaded if you aren't using it, and disabled the admin setting if the file doesn't exist.
I really like your callback mechanism and the data hooks. Looking forward to getting you onboard with this.

jvandervort’s picture

@kong, quick question for you, is there a way to pass data query params to the chart_data callback?
I didn't see any, so I'm curious how you do that.

kong’s picture

@jvandervort: OK I'll take a look at the 6.x-3.x-dev.

About the "data query params" I'm not sure what you mean. The chart_data hook's purpose is only to make sure that the function name in the URL is really the function intended to provide the chart data.

Or do you mean the ?ofc=data.php part like in the tutorials?

jvandervort’s picture

@kong, something like this:

url: /chart_1_page/%leaugue/%week

_data hook: return array('chart1_data', 'chart2_data', 'chart3_data');

chart1_data($league, $week) { ...}

chart_1_page($league, $week) {
   ...
  $output .= open_flash_chart_api_render('pickem_chart1_data')
}

How do I get my chart1_data callback to be called with the args I need to pass to it?
I can't see how to pass the args through _api_render back to my data callback function.

kong’s picture

Oh, I didn't think of that when I wrote the module. I guess we have to find a way to implement it then.

jvandervort’s picture

Just committed a first attempt, and modified the example chart to include args.

redndahead’s picture

Status: Active » Fixed

I think this thread has run it's course. @kong it's great to have you on board. Since we have a 6.x-3.x-dev Let's open new issues with this version.

kong’s picture

@jvandervort: the argument addition seems to work really fine.

Thanks to both of you, this project is getting better and better :)

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.