Implement resize callback for all charts

[johnSamilin]

fixes OHDSI/Atlas#1105

[chrisknoll]

@johnSamilin, this resize callback was to completely redraw the component so that the font sizes get rendered properly, and also the 'inset' of the legend can be applied differently based on the relative size of the legend and the overall graph size.

Just relying on the 'viewbox' property causes the text to get shrunk. Therefore, the callback is not redundant. Unless you can point me to the code where the render() method is called after a resize?

The isse described in OHDSIAtlas#1105 I believe comes from the incorrect width/height dimensions being fed into the control. Somehow it's sending in values that are larger than the actual component space and it causes that slight growth between renders.

[chrisknoll]

Here is the behavior of the current master branch:
Small:

Resized larger such that legend appears (inset):

Resized large enough to allow legend to appear outside the plot:

You'll have to open the above images in order to see 'actual size' but notice that between resizes that the font size remains consistent no matter how the plot is sized.

Now, with your PR, same resize steps:

Starting with default window size large (notice the legend is inset, and does not change):

Medium, fonts get smaller almost unreadable:

Small now, text is unreadable, the legened did not hide:

[johnSamilin]

@chrisknoll then why only line chart has this callback? what about the donut chart or any other? all of them have axis labels, and donut also has a legend

[chrisknoll]

True! This was just a focused fix to address some rendering issues related to line plot, but this could be hoisted up to the base 'chart' level and applied to all plots.

[johnSamilin]

Anyway, this should be done with css media queries and not resize callbacks. In most cases (like this one) I think it's anti-pattern

[chrisknoll]

I've tried it both ways, having the chart redraw itself has had the best result.

Some reading:
https://blog.webkid.io/responsive-chart-usability-d3/
http://bl.ocks.org/enactdev/a647e60b209e67602304

And i think this is a good example of why resize is better:
http://bl.ocks.org/josiahdavis/raw/7d84b2f1837eab9c24d9/

Starting with a default size:

Then scale it down. notice, fonts remain consistent, ticks are dynamic based on total width:

using media queries would not give you this effect: as you zoomed in, the font size would not adjust appropriately to pad the axis between the axis label, leading to overlapping text.

[johnSamilin]

Again, I don't mean only viewBox property, I'm talking about css media queries and using proper units. That would be enough for correct scaling

[chrisknoll]

Ok, if you think it's possible to have the same fidelity using those techniques, that would be great to see.

Still wondering how you can adjust ticks and orient axis labels based on dimensions via css and media queries, but I'm always willing to learn something new.

[chrisknoll]

Question:
couldn't we add the debounce implementation to the utils module of the library instead of making a new dependency on a 500k lodash library? the debounce is only 12 lines of code, is that worth making a dependency on a 500k size library?

[johnSamilin]

It gives us only 50 k of size

[chrisknoll]

Apolgoies! you are corect, this cdn shows just 23k download size. I forget where I saw a large bundle size on one of the other libs on atlas. But for this PR, the dependnecy on lodash isn't a large burdeon. Thanks. let me check the local tests for this change and let you know if there's any problems.

[chrisknoll]

@johnSamilin , sorry to distract this PR but i found where i saw lodash being so large:

This is the file that's downloaded with atlas. the difference is it is comming from a NPM install vs a CDN. Do you know if it is possible to get a smaller lodash lib from NPM that can be used in Atlas?

[chrisknoll]

@johnSamilin : i took the liberty of applying the resize logic across the different chart types (by hoisting the resize logic to the base chart class, and having subclasses call super()). A nice use of the class hierarchy! I need to import this component into my local atlas env to ensure nothing breaks, but I wanted to share these changes with you first.

[johnSamilin]

Do you know if it is possible to get a smaller lodash lib from NPM that can be used in Atlas?

In theory, it could be achieved by removing lodash dependency and introducing a number of atomic dependencies like lodash.flatten, lodash.find etc. I will check

[johnSamilin]

@chrisknoll it won't be effective because of two main reasons:

1. ohdsi.util.js uses almost all the lodash's functionality, so it will be loaded anyway
2. npm doesn't have all the modularized packages from lodash. for example, lodash.differenceWith and a bunch of others don't exist