DallasMorningNews / django-chartwerk

A Django-based application to manage, create and share Chartwerk charts.
https://django-chartwerk.readthedocs.io/
GNU Affero General Public License v3.0
8 stars 1 forks source link

Templatizing the embed code #45

Closed hobbes7878 closed 7 years ago

hobbes7878 commented 7 years ago

So I just met with my UX team, gave them the initial Chartwerk tour and talked about the embed integration for our CMS.

So ours is an interesting setup in that we bundle all the scripts for a page based on the content of that page beforehand. So for example, we would pre-bundle the script for embedding a video in a bundle with all other scripts for that page if the CMS detects a video in the story.

They'd like to do the same for Chartwerk's oEmbed integration. This would make the script tag that we normally include in the returned embed code redundant because that would be part of the integration CMS-side.

So this trips a wire I've been plucking for a while now: I wonder, rather than sticking another boolean on the config, should we collapse this need and the CHARTWERK_EMBED_SCRIPT config into some sort of templatized embed code.

So basically you provide a template string with whatever you need, and we render it and drop in some meta data.

Say we stipulate that your template string has to have at minimum a div with a class chartwerk attribute. We then append all the necessary data attributes for rendering the chart on that div and pass through whatever else you want in your template.

In this case, you could then actually inline the javascript that embeds the chart instead of needing to host it somewhere, if that's your bag. (Which would be a nice default for django-chartwerk out-of-the-box.) You could include pym if you wanted, for some reason. Whatever. etc. etc. etc.

Thoughts? @achavez

hobbes7878 commented 7 years ago

As a reminder, here is current state:

<div
  class="chartwerk"
  data-id="0j7g4uoh"
  data-embed="{&quot;single&quot;:{&quot;height&quot;:329,&quot;width&quot;:290},&quot;double&quot;:{&quot;height&quot;:494,&quot;width&quot;:600}}"
  data-size="double"
></div>
<script src="http://www.politico.com/interactives/charts/chartwerk/embed-script/v1.js"></script>
hobbes7878 commented 7 years ago

Last thing, I guess I'm actually suggesting the rendering would be part of chartwerk-editor. Django-chartwerk would simply pass through the template string.

hobbes7878 commented 7 years ago

Thinking through this more, we'd need to render the template string both server-side and client-side, b/c oEmbed.

Here's my proposal:

We allow you to customize the embed code returned by simply rendering a template string with predefined context. A valid template string would look like:

<div
  class="chartwerk"
  data-id="{{chart_id}}"
  data-embed="{{embed_sizes}}"
  data-size="{{preferred_size}}"
></div>

Context would look like:

{
  chart_id: "0j7g4uoh",
  embed_sizes: "{&quot;single&quot;:{&quot;height&quot;:329,&quot;width&quot;:290},&quot;double&quot;:{&quot;height&quot;:494,&quot;width&quot;:600}}",
  preferred_size: "double"
}

You would then specify your embed script in the embed code. Our full-featured default in django-chartwerk, for example, would like this:

<div
  class="chartwerk"
  data-id="{{chart_id}}"
  data-embed="{{embed_sizes}}"
  data-size="{{preferred_size}}"
></div>
<script>
(function(){
    var werks = document.querySelectorAll(".chartwerk");
    for (var i = 0; i < werks.length; i++) {
        var werk = werks[i],
            id = werk.dataset.id,
            dimensions = JSON.parse(werk.dataset.embed),
            size = werk.dataset.size,
            screen = werk.parentElement.clientWidth;
        // Check if iframe already embedded. (Handles for multiple embedded charts...)
        if (werk.querySelectorAll('iframe').length < 1) {
            var iframe = document.createElement("iframe");
            iframe.setAttribute("scrolling", "no");
            iframe.setAttribute("frameborder", "0");
            // double-wide
            if (size === 'double') {
                if (screen > dimensions.double.width) {
                    iframe.setAttribute("src", "http://yoursite.com/chartwerk/"+id+".html");
                    iframe.setAttribute("height", dimensions.double.height);
                    iframe.setAttribute("width", "100%");
                } else {
                    iframe.setAttribute("src", "http://yoursite.com/chartwerk/"+id+"_single.html");
                    iframe.setAttribute("height", dimensions.single.height);
                    iframe.setAttribute("width", dimensions.single.width);
                }
            // single-wide
            } else {
                iframe.setAttribute("src", "http://yoursite.com/chartwerk/"+id+"_single.html");
                iframe.setAttribute("height", dimensions.single.height);
                iframe.setAttribute("width", dimensions.single.width);
            }
            werk.appendChild(iframe);
        }
    }
})();
</script>

We scrap CHARTWERK_EMBED_SCRIPT from the config and add CHARTWERK_EMBED_TEMPLATE.

Let me know your thoughts. I'd like to work it in this weekend.

achavez commented 7 years ago

I've messed with this part the least, but from what I understand I'm in favor because of all the flexibility that it adds.

My one question would be about the need to render the map both client- and server-side. Would it be possible for the server to render and return to the client in a way that wouldn't break that workflow for the sake of having one consistent renderer?

hobbes7878 commented 7 years ago

That should be possible. I think we'd then want to add that logic onto the model, though, since we're using REST for the roundtrip on save. Custom method, I suppose, which would cleanup the oembed method a bit, too.

Sound right?

achavez commented 7 years ago

Yep. Sounds right to me.

hobbes7878 commented 7 years ago

Should we also let people add additional context to the embed, like CHARTWERK_EMBED_CONTEXT?

I can't think of a natural reason outside of replacing the http://yoursite... in the default template string, but maybe that's enough. ..

achavez commented 7 years ago

Would this be something where they'd give us a dict() to pass in that we'd merge with our default context?

Or maybe they'd provide a function, we'd pass in the default context and allow them to add to it/modify it and return a django.template.context.Context?

hobbes7878 commented 7 years ago

I was thinking the former, but I'm intrigued by the latter. What would you do with it?