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

Added HighMaps #92

Merged
merged 18 commits into from
Jun 16, 2018
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 49 additions & 5 deletions lib/daru/view/adapters/highcharts/display.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
require_relative 'layout_helper_iruby'
require_relative 'iruby_notebook'
require 'daru/view/constants'

module LazyHighCharts
def self.init_script(
dependent_js=['highstock.js', 'highcharts-more.js', 'modules/exporting.js',
'highcharts-3d.js', 'modules/data.js']
dependent_js=[HIGHSTOCK, MAP, EXPORTING, HIGHCHARTS_3D, DATA]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGHSTOCK, MAP, EXPORTING, HIGHCHARTS_3D, DATA] is also constant as web app highcharts dependencies.

)
# Highstock is based on Highcharts, meaning it has all the core
# functionality of Highcharts, plus some additional features. So
Expand Down Expand Up @@ -33,22 +33,25 @@ class HighChart
#
def to_html(placeholder=random_canvas_id)
chart_hash_must_be_present
script = load_dependencies('web_frameworks')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So now each time dependencies will be loaded, whenever to_html is invoked. Can we control this loading (it will load once), using Proxy design pattern?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to_html will load the dependencies only if there is some additional dependency provided in the modules option or if a map is being drawn (to load the required map data). Otherwise, load_dependencies will simply return an empty string. I will look into the Proxy design pattern also.

# Helps to denote either of the three classes.
chart_class = extract_chart_class
# When user wants to plot a HighMap
if chart_class == 'Map'
high_map(placeholder, self)
script << high_map(placeholder, self)
# When user wants to plot a HighStock
elsif chart_class == 'StockChart'
high_stock(placeholder, self)
script << high_stock(placeholder, self)
# When user wants to plot a HighChart
elsif chart_class == 'Chart'
high_chart(placeholder, self)
script << high_chart(placeholder, self)
end
script
end

def show_in_iruby(placeholder=random_canvas_id)
# TODO : placeholder pass, in plot#div
load_dependencies('iruby')
IRuby.html to_html_iruby(placeholder)
end

Expand All @@ -61,6 +64,47 @@ def to_html_iruby(placeholder=random_canvas_id)
high_chart_iruby(extract_chart_class, placeholder, self)
end

# Loads the dependent mapdata and dependent modules of the chart
#
# @param [String] to determine whether to load modules in IRuby or web
# frameworks
# @return [void, String] loads the initial script of the modules for IRuby
# notebook and returns initial script of the modules for web frameworks
def load_dependencies(type)
dep_js = extract_dependencies
if type == 'iruby'
LazyHighCharts.init_iruby(dep_js) unless dep_js.nil?
elsif type == 'web_frameworks'
dep_js.nil? ? '' : LazyHighCharts.init_script(dep_js)
end
end

# Extracts the required dependencies for the chart. User does not need
# to provide any mapdata requirement explicity in the `options`.
# MapData will be extracted using `options[:chart][:map]` already
# provided by the user. In `modules` user needs to provide the required
# modules (like tilemap in highcharts) in the form of Array. Once the
# dependency is loaded on a page, there is no need to provide it again in
# the `modules` option.
#
# @return [Array] the required dependencies (mapdata or modules)
# to load the chart
# rubocop:disable Metrics/AbcSize
def extract_dependencies
dep_js = []
# Mapdata dependencies
if !options[:chart_class].nil? && options[:chart_class].capitalize == 'Map'
dep_js.push('mapdata/' + options[:chart][:map].to_s + '.js') if
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

user input countries/folder1/folder2 things is making it complicated. Think about other solution.

Copy link
Contributor Author

@Prakriti-nith Prakriti-nith May 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have kept the same folder structure provided in the highcharts dependencies. It is because the value provided by the user (it is in highcharts option and will be provided everytime user needs some mapdata dependency) in options[:chart][:map] will be the same as of the dependencies and by this folder structure it will be easy to retrieve the data.

options[:chart] && options[:chart][:map]
end
# Dependencies provided in modules option (of highcharts mainly
# like tilemap) by the user
dep_js |= options.delete(:modules).map! { |js| "#{js}.js" } unless
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think , options created by us (like chart_class, modules, map ) must be in separate parameter. Only highcharts options must be passed as 2nd parameter. WDYT?

Copy link
Contributor Author

@Prakriti-nith Prakriti-nith May 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

map option is not created by us and will be provided in highcharts' options everytime there is a need for map data(like europe.js). This would be the exact address of the dependency required.
For example, a user using dependency https://code.highcharts.com/mapdata/countries/in/in-all.js will provide countries/in/in-all as a value for options[:chart][:map].
Only the options chart_class and modules are created by us. User does not need to provide any additional option for mapdata.
Yes, we can have a third parameter for these but IMO that would be only more complex for the user plus we then have to generalize it for other adapters as well which I think is not a good option.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User will put path for the js to load in modules options (indirectly), right? Any other use case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modules option is created just to load some additional dependencies used in a specific chart by the user.

options[:modules].nil?
dep_js
end
# rubocop:enable Metrics/AbcSize
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try to fix rubocop error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I tried removing it but the conditions are necessary. Please provide any suggestions to fix it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess, it must have method to check for map chart and chart class. if condition inside if condition looks bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it earlier but still, there was rubocop error. I have separated out the method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO all the four conditions are necessary. Which one should I remove?


# @return [String] the class of the chart
def extract_chart_class
# Provided by user and can take two values ('stock' or 'map').
Expand Down
8 changes: 6 additions & 2 deletions lib/daru/view/adapters/highcharts/iruby_notebook.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require 'daru/view/constants'

module LazyHighCharts
# generate initializing code
def self.generate_init_code(dependent_js)
Expand All @@ -9,15 +11,17 @@ def self.generate_init_code(dependent_js)

# Enable to show plots on IRuby notebook
def self.init_iruby(
dependent_js=['highstock.js', 'highcharts-more.js', 'modules/exporting.js',
'highcharts-3d.js', 'modules/data.js']
dependent_js=[HIGHSTOCK, MAP, EXPORTING, HIGHCHARTS_3D, DATA]
)
# TODO: include highstock.js for highstock and modules/*.js files for
# exporting and getting data from various source like csv files etc.
#
# Highstock.js includes the highcharts.js, so only one of them required.
# see: https://www.highcharts.com/errors/16
#
# Using Highmaps as a plugin for HighCharts so using map.js instead of
# highmaps.js
#
# , 'modules/exporting.js' : for the exporting button
# data.js for getting data as csv or html table.
# 'highcharts-more.js' : for arearange and some other chart type
Expand Down
8 changes: 8 additions & 0 deletions lib/daru/view/adapters/highcharts/layout_helper_iruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@ def high_chart_iruby(chart_class, placeholder, object, &block)
).concat(content_tag('div', '', object.html_options))
end

def high_map(placeholder, object, &block)
object.html_options[:id] = placeholder
object.options[:chart][:renderTo] = placeholder
build_html_output(
'Map', placeholder, object, &block
).concat(content_tag('div', '', object.html_options))
end

private

def build_html_output_iruby(type, placeholder, object, &block)
Expand Down
Loading