-
Notifications
You must be signed in to change notification settings - Fork 20
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
Handling events in GoogleCharts #100
Handling events in GoogleCharts #100
Conversation
Pull Request Test Coverage Report for Build 635
💛 - Coveralls |
@Shekharrajak can you please review this PR and let me know if there are any changes that I have to make? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separate the other PRs code from this
js << "\n \t\toptions: #{js_parameters(@options)}," | ||
js << "\n \t\tcontainerId: '#{element_id}'," | ||
js << "\n \t\tview: #{extract_option_view}" | ||
js << extract_chart_wrapper_options(data, element_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ChartWrapper here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add_listeners_js('wrapper')
in this method was generating rubocop error so I've created another method extract_chart_wrapper_options
in the common module display.
Please write examples which can be useful in real world application. |
3de4278
to
454c58b
Compare
Right now, I have added these examples. |
spec/adapters/googlecharts_spec.rb
Outdated
) | ||
} | ||
let (:table_spreadsheet) { | ||
Daru::View::Table.new( | ||
data_spreadsheet, {width: 800} | ||
) | ||
} | ||
let(:data_table) {Daru::View::Table.new(data)} | ||
let(:user_options) {{chart_class: 'Chartwrapper'}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better if you write new variables for new specs, rather than modifying existing variables completely.
@Shekharrajak can you please review this PR? |
6ede940
to
14ecc63
Compare
sh "curl -# http://www.google.com/jsapi -L --compressed -o lib/daru/view/adapters/js/googlecharts_js/google_visualr.js" | ||
end | ||
end | ||
# FIXME: Updating jsapi is causing error in IRuby notebook and Googlecharts do not work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open an issue to track this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
task :loader do | ||
say "Grabbing loader.js from the google website..." do | ||
sh "curl -# http://www.gstatic.com/charts/loader.js -L --compressed -o lib/daru/view/adapters/js/googlecharts_js/loader.js" | ||
end | ||
end | ||
|
||
task :jspdf do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we using it ?
With this feature, a user can apply multiple listeners to a google chart/table to handle various events. I have added jspdf dependency for web frameworks because a user can at least use
ready
event to export the chart in pdf format. There are fewer examples provided on the official website. Examples.Event handling can be done in Highcharts easily through the highcharts options like this, so in googlecharts also, we are handling events through options (in third parameter).
Syntax:
Provide this in user_options (third parameter)