(Reopening) Chartkick is not defined with turbolinks defer

[feliperaul]

Hi @ankane!

As usual, thank you so much for your OS contributions, they are huge.

I'm also having problems with Chartkick and turbolinks with defer. Everything else works just fine, but Chartkick raises that JS exception when the page is opened through clicking a link (it works, tough, when refreshing the page directly in the chartkick view).

In https://github.com/ankane/chartkick/issues/480#issuecomment-498919626 (#480) you mentioned that there's an edge branch with a solution, but that branch is currently 17 commits behind master.

Do you plan in releasing that fix in the master branch?

Thanks!

[ankane]

I'm not sure if Chartkick will move to the pattern in edge, but in the meantime, we can add a defer: :turbolinks option. Can you give the defer_turbolinks branch a shot?

[feliperaul]

@ankane Checking on it right now.

[feliperaul]

@ankane Sent a pull request

[ankane]

Alright, take 2 (based on the discussions in #537).

turbolinks branch - https://github.com/ankane/chartkick/compare/turbolinks

• turbolinks: true cleans up the charts automatically and (at the moment) disables preview
• turbolinks: true, defer: true does the above and defers the chart loading until the turbolink:load event

Let me know what you think.

[feliperaul]

@ankane Perfect for me!

[ankane]

Awesome. For the preview, I think a few options are:

1. Don't show it. Charts show up as empty spaces. (current code)
2. Run the original JS. Charts show up, but animations run and data from urls is refetched. (doesn't seem good)
3. Run separate JS to render the chart without animations and with cached data.

I think 3 would be most like the preview for other elements, but adds the most complexity, so 1 may be the best option for now.

[feliperaul]

@ankane I think 1 is a great improvement from master, since it fixes the defer problem (there were at least 4 issues that were opened regarding this over time) AND it also handles the turbolinks cleanup which wasn't being done before.

In the future someone might tackle the preview, but the UX is great the way it is.

Just one last thing that came to mind: can you test using remote urls (https://github.com/ankane/chartkick#say-goodbye-to-timeouts) ? I'm not able to test it right now, but just to be sure this branch doesn't break anything and it also works with data fetched after initial page load.

[feliperaul]

@ankane Just to clarify, when I said the 'UX is great the way it is', I meant 2 things, (1) it behaves just like the initial page load, where we have an empty space in the page before the charts kick in, so there's no regression and (2) even with complex graphs, it happens so quickly in my tests I'd call it instant, so I can't even notice that white space.

[ankane]

Hey @feliperaul, think I found a better pattern to address this for Chartkick 4 (which makes all charts deferrable by default) if you want to try it.

gem 'chartkick', github: 'ankane/chartkick', branch: 'deferrable'

And add to your JavaScript file after Chartkick.js is required:

// will be part of Chartkick.js 4.0
window.dispatchEvent(new Event("chartkick:load"));

// will be part of Chartkick.js 4.0
Chartkick.destroyAll = function() {
  for (var id in Chartkick.charts) {
    Chartkick.charts[id].destroy();
    delete Chartkick.charts[id];
  }
}

// may be part of Chartkick.js 4.0 or may leave it to apps to add
document.addEventListener("turbolinks:before-render", function() {
  Chartkick.destroyAll();
});

[ankane]

Hey @feliperaul, just released Chartkick 4, which makes charts deferrable by default and works with Turbolinks (including chart clean up).

[ankane]

Thanks for bringing this up and helping with it!