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

add ability to use listeners #39

Merged
merged 2 commits into from
Oct 16, 2012
Merged

add ability to use listeners #39

merged 2 commits into from
Oct 16, 2012

Conversation

greinacker
Copy link
Contributor

Adds the ability to add listeners to charts. I realize you said this in the readme file:

For example, Methods and Events as described in Google Chart Tools’ API Docs, for use after a chart has been rendered, are not implemented because they felt more native being written as JavaScript functions, within views or .js files.

This totally makes sense; however, there isn't an easy way to "wire up" these javascript listeners when using this gem. The attached change is intended to be a generic way to attach callbacks to these charts, without (IMHO) violating the spirit of what you're talking about in the readme...

ref issue #38

@travisbot
Copy link

This pull request passes (merged 83d12c7 into 13c76cb).

@greinacker
Copy link
Contributor Author

Some sample code that uses this:

my_chart.add_listener("select", "function() {chart_select_event(chart);}")

So not only can we add listeners, but we can also use the "chart" variable that's only available within the per-chart script, allowing us to use multiple charts on one page and be able to add listeners for each of them. In my own code, I also pass an ID identifying which chart it is to the callback, but this example gets the point across...

@szymon-jez
Copy link

+1
@greinacker This could not be done better. Please also don't forget to write about it in the documentation (README) ;)

@winston
Copy link
Owner

winston commented Oct 15, 2012

Happy to merge this in. But I am a little sad that it's not tested. =)

Let me know if you have time to get some tests in, else I'll merge it anyway and add in the tests in my free time.

Thank you!

@greinacker
Copy link
Contributor Author

I too was a little sad when you pointed that out. Spec added. :-)

winston added a commit that referenced this pull request Oct 16, 2012
add ability to use listeners
@winston winston merged commit fd81c0b into winston:master Oct 16, 2012
@fonteijne
Copy link

Hey there, I this works really easy. Just add one line and there is an event-listener! :)
The only thing I'm missing is some documentation for this. Am I just overlooking this, or is it really missing?
I would really want to get the selection.column name when I click on a table-field.
Is this already possible?
When calling this line:
"var selection = visualization.getSelection();" (from the Google Playground) it just stops.
So, is it possible to write some documentation for this feature?
Thank you in advance.

Fonteijne

@kandadaboggu
Copy link
Contributor

@fonteijne refer to this reply for a sample usage: #36 (comment)

@winston
Copy link
Owner

winston commented Nov 12, 2012

You are right about the lack of documentation. Sorry about that.

I've raised a feature request fro it over on the docs repo. Will get to it soon.

winston/google_visualr_app#1

Thank you.

@fonteijne
Copy link

@kandadaboggu Thank you for the reference. Still struggling, but getting further.
I already launched an alert by clicking the graph. The graph is fully functioning.
But a table on the same page(!) is not doing the thing. Everywhere I click is '0'.
-- update
It seems like the wrong data_table is loaded. It (always) returns the value(!) of the first column. So, not the columnname, but the value of the first column.

@winston Thank you for the feature request. When I got it all figured out, maybe I can find the time to write a concept documentation for it. I'll keep you posted.

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

Successfully merging this pull request may close these issues.

6 participants