Introducing iframe elements on v2

[heruan]

Since I'm planning to switch to v2 soon, I'm preparing a major update of our apps and I tried v2.0.0-beta2: suddenly a wild iframe appeared in the DOM (causing issues with AdBlock, BTW).

Is v2 really introducing an iframe element when drawing graphs?

[etimberg]

@heruan v2 is adding an iframe. The reason an iframe is added is to detect when the canvas container resizes. There is a window resize event, which was used by v1, but it only detects when the entire window resized and not a specific element. This led to the problems of charts not displaying correctly when their container was initially display: none; and then displayed (See #762 ).

I investigated a number of solutions for implementing better resize behaviour. Almost all of the available solutions use some combination of a timer / requestAnimationFrame. Unfortunately, this is bad for mobile battery life since we have to poll the size of the container continuously and check for a change.

The iframe solution works because it is set to be the same size as the canvas container node. Since the iframe is a window, the window resize event can be listened to and then passed up out of the iframe to the chart to tell it when the container node resized.

There is some other discussion on this topic in #2024

[heruan]

Thank you @etimberg for the clear explanation, it doesn't sound that bad for this purpose but unfortunately I'm having resize issues when using v2 in Single Page Applications based on Angular 2 and Aurelia. May it be because they build the DOM dynamically, injecting/removing elements while the user navigates the app? I'll build up a sample project and open separate issues!

[etimberg]

@herun if you could build up a sample project, that'd be great! I'm wondering if we can do the dom addition in a callback. The users could provide an overridden implementation that works with their framework (angular, react, etc)

[heruan]

Issue reference: #2212

[sknick]

Just to add my $0.02 on this: The hidden iframe causes some issues for me when integrating Chart.js v2 into my Echo 3-based client-side application. Specifically, the insertion of the hidden iframe just before my canvas node prevents the canvas element from rendering properly (its height and width are left unset). My workaround is to disable resize listening:

Chart.defaults.global.responsive = false;

[decherneyge]

Continuing with what @sknick did I took it one step further and decided to use the global flag as the kill switch for the iframe. I read the comment in the src about binding so the responsiveness could be changed dynamically but in my case I cared less about that and more about the performance gain* I would receive by not having the iframe.

var origResizeListener = Chart.helpers.addResizeListener;
Chart.helpers.addResizeListener = function(node, callback){

    if(Chart.defaults.global.responsive){
        origResizeListener(node,callback);
    }

};
Chart.defaults.global.responsive = false;

*Under "normal usage" I never saw a performance problem but I have an edge case scenario. We have 100+ little doughnut charts on the page and if someone were to change the filter on the data source it was taking much longer to reload than I was expecting. I knew it wouldn't be pretty I was surprised at the actual slowness. I noticed the favicon constantly flickering and started digging in. That's when I saw the iframe and tried to turn it off which lead me here.

[ghost]

The hidden iframe introduces an accessibility issue as it raises the warning AX_FOCUS_01 "These elements are focusable but either invisible or obscured by another element". I think the warning may be mitigated by adding aria-hidden="true"to the iframe element.

[simonbrunel]

@MetaTribal where can I see that accessibility warning?

[ghost]

I'm using the Google Accessibility Tool, here:
https://chrome.google.com/webstore/detail/accessibility-developer-t/fpkknkljclfencbdbgkenhalefipecmb?hl=en

On Sep 4, 2016 11:00 AM, "Simon Brunel" notifications@github.com wrote:

@MetaTribal https://github.com/metatribal where can I see that
accessibility warning?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2210 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFAjfMlnAMBIaD6rxfLx7u1BLxzoY4aDks5qmwdBgaJpZM4H-k2J
.

[simonbrunel]

Thanks :)

[simonbrunel]

I can't reproduce the accessibility issue, maybe it needs some specific context:

[ghost]

I'm traveling right now; but will look at my laptop next time i get a
chance. I ran across this while putting together an accessibility report.
There may have been specific context or tooling involved.

On Sep 4, 2016 11:42 AM, "Simon Brunel" notifications@github.com wrote:

I can't reproduce the accessibility issue, maybe it needs some specific
context:

[image: image]
https://cloud.githubusercontent.com/assets/3874900/18233169/013d46e0-72e0-11e6-96f5-7f409afb57c9.png

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2210 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFAjfOhMSogtM-r6m-N2eSfe2blmrq3Uks5qmxEVgaJpZM4H-k2J
.

[heruan]

I'm reopening this since the use of an iframe to get responsiveness is semantically wrong and it's resulting in more than one side effect. I understand @etimberg reasons, but in my opinion a library well-written like chart.js should respect standards and accessibility and have tricks to enable some extra functionality available as an opt-in (e.g. enabled by an option).

By default, there should be no wild iframes nor listeners since the developer have many ways to keep track of the canvas size and decide what to do and when.

[simonbrunel]

Agree with you @heruan, but the default behavior can't be changed because it would break too many projects (at least for version 2.x).

However, we are working on 2 main changes about chart responsiveness: at very short term (certainly v2.3.1), the iframe will not be injected anymore if responsive: false (not the default config, but will solve your issue). We are also investigating a method to replace the troublesome iframe by a group of div to detected size changes (probably v2.4), so most of issues related to performance, accessibility, standards, ad blockers, etc., because of the iframe, should disappear.

[simonbrunel]

Fixed by #3364 (no wild iframe if responsive: false - not the default) and available in v2.4.0

[johnmanko]

Might need to reopen this. I have a chart in a scrollable div, and when the iframe is included in the DOM the canvas scrolls but nothing else does. When I manually remove the iframe from the DOM everything scrolls with the div correctly. I have a fixed height on a parent element, with 100% width. Unfortunately, setting the responsive: false option prevents the width from adapting to 100% of the view so I can't just set that to false. This was not a problem with version 1.x.

Before scrolling with iframe:

After scrolling with iframe (bad):

Scrolling without iframe (good):

[simonbrunel]

@johnmanko can you provide a live example of this issue?

[johnmanko]

After additional testing, it looks like this may be a problem with a specific version of Chrome.
Version 53.0.2785.143 Built on Ubuntu , running on Ubuntu 16.04 (64-bit)

The following version of Chrome everything works as it should:
Version 54.0.2840.99 m

It works on Firefox.