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

Global options #24

Closed
cseelus opened this issue Aug 4, 2019 · 9 comments · Fixed by #39
Closed

Global options #24

cseelus opened this issue Aug 4, 2019 · 9 comments · Fixed by #39
Labels
enhancement New feature or request

Comments

@cseelus
Copy link
Contributor

cseelus commented Aug 4, 2019

First, thanks @styd for this awesome wrapper.

Out of curiosity, I tried to replace Chartkick in one of our projects and for the reports I tried it with, it was almost like using a drop-in-replacement. Only that the Apexcharts look better :-)

When using this in production, one would most likely want to have global options, like disabling the toolbar, setting a font and theme for all charts and so forth.

I think (for a Rails app) an initializer would be the way to go:

Apexcharts.options = {
  chart: {
    animations: {
      enabled: false,
    },
    fontFamily: '"CGE Sans", Helvetica, Arial, sans-serif',
    toolbar: false,
    
  },
  
}

These global options hash would then be merged with the options argument per Apexcharts helper call:

<%= line_chart(series, options) %>
                       ^^^^^^^

Of course this could all be done manually, but having distinct global options would be cleaner and safer (no maintainer could forget to manually merge the global options).

@cseelus cseelus added the enhancement New feature or request label Aug 4, 2019
@styd
Copy link
Owner

styd commented Aug 4, 2019

Hi, @cseelus. Thanks for the kind word. :-)

That is a really good idea and it's actually quite easy to implement. ApexCharts.js already has a global variable called Apex. On the browsers this will be window.Apex.

This variable will be merged with options on every chart by ApexCharts.js. So, we just need to make sure that this variable is loaded before the first chart on every page that contains the chart. I'm thinking of reading the apexcharts_id, if it's 1 then load the window.Apex first.

Do you think you can make a pull request for this? After having my youngest son being sick for almost a week, I just found out that my old and only laptop was broken. So, last week was a very sad week for me. If you can make a pull request for this, I'll really appreciate it and I'll guide you. But don't sweat it if you can't. I'll implement this probably next week after getting my laptop fixed or a new laptop.

@cseelus
Copy link
Contributor Author

cseelus commented Aug 4, 2019

That sounds awful, hope your son is 100% well again and your laptop can be fixed!

You are completely right though, ApexCharts itself already supports global options in a very convenient way.
After some digging, I found out that one can use the Ruby hash from my first post as a JavaScript object and just merge it with the Apex object (after the apexcharts JS was required) with Object.assign(Apex, myGlobalOptionsObject). Simple as that.

So I guess there is no need from ApexCharts.rb to support global options separately?

@styd
Copy link
Owner

styd commented Aug 5, 2019

Thanks. I hope so too.

Well, that works too. But not in a ruby way. And you still have to explicitly do that before every first chart. So, kind of back to square one I guess. Don't worry, I'll do it later.

@cseelus
Copy link
Contributor Author

cseelus commented Aug 5, 2019

Defined once after apexcharts.js is included, the options work for the whole app, as they are merged with the global Apex JS object from which every chart is created.

I thought merging the user defined (global) options with that object was your initial idea, but apparently you meant to do it differently. What other approach are you suggesting?

Edit: My initial idea was to use a Rails initializer to define global options as a hash and merge them with the options defined per helper call within ApexCharts:: OptionsBuilder for each chart helper that is used. Manipulating the global Apex object directly seems more elegant and efficient though.

@styd
Copy link
Owner

styd commented Aug 5, 2019

Defined once after apexcharts.js is included, the options work for the whole app, as they are merged with the global Apex JS object from which every chart is created.

Unless you modify the apexcharts.js file by adding window.Apex assignment, the Apex options will be lost on another page that requires (or adds script that sources) the apexcharts.js.

I was thinking of these steps:

  1. Set the default value for ApexCharts.options to {} (ApexCharts::Helper might be a good place to do this) so that users can assign ApexCharts.options before calling the charts. Initializers sounds like a good place for users of Rails.
  2. Add another flag (we can make another options key, say load_apex, with boolean value) that tells the renderer to add the following html before the html that adds charts container and render them when the apexcharts_id equals 1:
"<script>window.Apex = #{ApexCharts.options.to_json}</script>" if options[:load_apex] && !ApexCharts.options.empty?

What do you think?

@cseelus
Copy link
Contributor Author

cseelus commented Aug 5, 2019

Unless you modify the apexcharts.js file by adding window.Apex assignment, the Apex options will be lost on another page that requires (or adds script that sources) the apexcharts.js.

I not sure why that would be the case?

We use a JS file that includes the apexcharts.js and sets global options for it. The author of Apexcharts JS himself recommends this approach. Works on all pages of our app where charts (sometimes multiple charts) are rendered, with or without Turbolinks:

//= require apexcharts

apexGlobalOptions = {
  chart: {
    animations: {
      enabled: false,
    },
    fontFamily: '"CGE Sans", Helvetica, Arial, sans-serif',
    toolbar: false,
     some more options
  },
  theme: {
     some more options
    }
  },
   some more options
}

// Merge global Apexcharts options with Apex object
Object.assign(Apex, apexGlobalOptions)

Am I missing a use case where this would not work?

Regarding your proposed approach: I don't get Step 2, adding the <script> into HTML when apexcharts_id equals 1. Why not just merge the user defined global options with the options from each helper?

<%= line_chart(series, options) %>
                       ^^^^^^^

Until I manipulated the Apex JS object directly as shown above, I was using a similar approach manually:

# global_chart_options defined as a helper
<%= line_chart(series, global_chart_options.merge({ some_local: options }) %>

@styd
Copy link
Owner

styd commented Aug 5, 2019

Ah, my bad. I misunderstood greatly. I was thinking of a more general solutions that works for rails or any other framework (even plain .erb) out of the box without modifying the .js too much. Except that for plain .erb, it needs to require '.rb' or load a '.yaml' file for the global options.
I guess your solution works great for Rails users.
The reason I don't want to merge the global options on every chart is simply because of more work. What if there are tens of charts on a page.

@cseelus
Copy link
Contributor Author

cseelus commented Aug 5, 2019

No problem, that is understandable ;-)

@cseelus cseelus closed this as completed Aug 5, 2019
@styd
Copy link
Owner

styd commented Aug 6, 2019

Let's keep this feature request open as it can be a nice to have feature to implement in ruby. Maybe we can make it pick up the global options from a yaml file based on the environment or something.
Besides, people reading our conversations can know why this project hasn't been updated for more than a week and possibly for another week.

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

Successfully merging a pull request may close this issue.

2 participants