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

Introduce support for chart resizing #544

Open
wssbck opened this issue Mar 19, 2014 · 47 comments
Open

Introduce support for chart resizing #544

wssbck opened this issue Mar 19, 2014 · 47 comments
Milestone

Comments

@wssbck
Copy link
Contributor

wssbck commented Mar 19, 2014

I have hit an issue when trying to use DC charts in a responsive layout. Since charts do not automatically redraw to accommodate the new size of their HTML container, I am forcing them to do so by calling dc.renderAll() on every window resize event. This is however not a good solution because it uses a lot of resources and some charts do not render properly during resizing anyway. For example, bars in row charts seem to "forget" that the HTML container has shrinked horizontally and still render like it has not.

@axiomabsolute
Copy link

I recently started using dc.js on a project that also needed a responsive layout. The project was already using d3.js for less interactive visualizations, where we settled on using the svg viewBox attribute to achieve the required resize behavior. It's not a perfect solution, since things like the axis and legend will scale along with the chart itself, which can look awkward, but it's a fairly easy and performant work around from what I've seen.

Here's a jsfiddle showing the technique. Resize the results pane to see it in action.

There's a slight flicker when the page first loads before the postRender listener completes. This can be mitigated somewhat by setting the initial chart height to zero then using an animation library to fade it in at the new hight.

Looking at the source code for dc.js, I don't think it would be very difficult to add a fitParent(bool) function to the BaseChart mixin to modify the rendering behavior to use the width and height already set on the chart as the viewBox bounds and then actually render with percentage-based (100%, to be exact) height and width, like in the fiddle.

If the awkwardness of the large axes/legend are not your thing, you could always stick with your current technique, with a couple of tweaks to improve performance, like using the dc.events.trigger(..) function to ensure that the calls to renderAll() don't fire too fast for the UI to keep up.

@jefffriesen
Copy link

I don't think it would be very difficult to add a fitParent(bool) function to the BaseChart mixin to modify the rendering behavior to use the width and height already set

Or another approach would be to detect a % inside .height() and .width() and if so, size them that way.

.width(300)    // 300px
.width('80%')  // 80% of parent

Right now I have a function that I pass to the width:

.width(_.span(6))    // corresponds to the width of .col-md-6 in Bootstrap 3

Then on window.resize() I .redrawAll().

But passing in a percentage to width would be much cleaner and intuitive.

@axiomabsolute
Copy link

@jefffriesen I don't think just providing .width(80%).height(80%) is enough to use this technique; we need a base size for the viewBox to define the initial aspect ratio. If we simply redraw the graphic to fit the parent without considering the aspect ratio we're likely to end up with very distorted, squished results unless the parent happens to be at the exact same aspect ratio. On the other hand, I don't think assuming a base aspect ratio is a good idea either, based on the variety of common aspect ratios for DC's charts, e.g. a pie chart typically has a rather square bounding box (unless there's a legend throwing things off), while a bar chart is rectangular.

Instead, I think the best way to keep it intuitive is by translating the behavior of fitParent to the underlying SVG concepts as transparently as possible; calling fitParent(true) uses the height/width of the chart as the bounds of the viewBox and uses the parent's height/width as the height/width of the SVG.

I've forked the master branch and started making the changes I've described above to add support for the technique I mentioned above. It essentially leaves the height-width of the chart in place, meaning that it doesn't break any of the existing charts which rely on them for computing internal rendering coordinates, and instead simply changes attributes on the generated SVG element.

EDIT: The relevant changes without documentation

@gordonwoodhull
Copy link
Contributor

@axiomabsolute, note we also have the (fairly new) behavior of fitting to the parent div if width or height is set to null, which seems to be similar to what you're proposing with fitParent.

https://github.com/dc-js/dc.js/blob/master/web/docs/api-latest.md#heightvalue

The parent div can in turn be sized at a percentage of its parent, so I don't think anything needs to be built in for percentages.

I poked around to see if there are resize events on elements. It seems there is only a browser-level resize event, and then some hacks involving overflow detection which I'd be wary of including. Not sure how to address that part.

@gordonwoodhull
Copy link
Contributor

I think the bar width resize problem @wssbck mentions is the same as #572 / #533.

Generally people seem to recommend throttling events using something like dc.trigger because yes, it is rather resource-intensive to redraw elements as fast as the mouse can resize the window.

@axiomabsolute
Copy link

@gordonwoodhull Thanks, I hadn't noticed that feature was added! Combining that with adding the viewBox attribute to the parent SVG in the postRender handler is a pretty solid work around in my case, since it gets rid of the flickering effect completely while keeping the live-resize functionality.

Maybe use the viewBox for the live resize portion then fire off a true redraw only when the resize event is "complete" (i.e. the user hast let go of the mouse button)? This would give a kind of live preview of the chart with a relatively low performance cost, but would still do a true redraw at the end to ensure that elements that SHOULDN'T be scaled (like the axis markers and text labels) aren't distorted. You could even toss in the dc.trigger functionality so that if the user pauses for long enough while resizing it would trigger a full redraw.

@gordonwoodhull gordonwoodhull added this to the v2.1 milestone Jun 5, 2014
@gordonwoodhull
Copy link
Contributor

Related (not an answer, just cultivating the question):
https://groups.google.com/forum/m/#!msg/dc-js-user-group/U7YVh6aVwNg/AOMscghZetoJ

@gordonwoodhull
Copy link
Contributor

@neverfox
Copy link

I made the parent responsive width (and set the chart width to null), then I'm using a resize event to rerender (with trigger delays), but while it works sometimes (and the svg element gets correctly sized all the time), the internal elements (like g elements) get "out of sync". They're either left at the larger size (and get cut off) or they stay at the smaller size even though the svg grows.

@ghost
Copy link

ghost commented Oct 7, 2014

I'm currently using the height(null) and width(null) to get responsive rectangular charts but I can't find any documentation on how to get round pie charts to have dynamic radius. radius(null) doesn't seem to work. Any plans on getting the same functionality for radius?

@gordonwoodhull
Copy link
Contributor

@alexhultman, could you open a separate issue for this? It looks like it is supposed to work:

https://github.com/dc-js/dc.js/blob/master/src/pie-chart.js#L85

    function drawChart() {
        // set radius on basis of chart dimension if missing
        _radius = _radius ? _radius : d3.min([_chart.width(), _chart.height()]) / 2;

Please include the error you're getting, if any.

@ghost
Copy link

ghost commented Oct 8, 2014

Thanks for the quick reply. It actually seems to work. However, it doesn't redraw on resize like the other charts we have. I have to reload the page to get it to redraw. Could be our fault though. Anyhow - could you please update the docs about radius(null)? Thanks.

@gordonwoodhull
Copy link
Contributor

Ok, I changed the description of .radius. It'll be updated next time I generate the docs.

I am not sure, but I think if that if any charts redraw on resize, or even automatically rescale when redrawn and their div has changed size, it is accidental. That's what this ticket is about. I have not investigated this, however.

It would be nice to support and PRs are always welcome!

@mberhorst
Copy link

I have the same behaviour like neverfox. When resizing the window I set a new height and width to the charts and use dc.renderAll(). The svg gets the new dimensions but the g-element inside the svg does not resize. Linecharts however resize correctly. RowCharts not.

When I destroy all charts and render them from scratch the new width/height is set correctly. At the end I render everything from scratch. It would not be the best solution but how often does the user resize the viewport? I think only developers do this that often to test the ui.

@davisford
Copy link
Contributor

This isn't perfect, but I pasted a quick gist on how I've been doing this for many months now, and it is working without issue. We use a ton of row, bar, pie, and line charts, and it is all working with bootstrap at all the various breakpoints.

This is angular specific (sorry), but you could extract the main logic out of it. The solution is essentially to debounce on window's resize event and set the svg width attribute to 100%, then fire either rescale() and/or redraw().

This isn't the best way to build an angular directive either (shared scope with controller), but I figured I'd just post it if someone finds it useful. I don't attempt to resize the height, as we don't really want that with our charts. If you really want to resize the height and width, I'd use the svg viewport to scale it.

@mberhorst
Copy link

Thanks. I will give it a try but svg-viewport is not a solution for me. I tested it and it looks awful when everything is scaling including the axis.

@gordonwoodhull
Copy link
Contributor

Support for resizing coordinate grid charts is in 2.0.0 beta 15. You still need to drive it from some event you subscribe to (like window.onresize) but setting the width and height and then doing a redraw works (no render required).

Also moving this goal to 2.0 because it seems these changes can be made without breaking the API.

I'm also adding a new examples directory, mostly for testing these:
http://dc-js.github.io/dc.js/resizing/
(will be active once I push beta 15 later today)

We can continue to add charts there as they gain compatibility.

@gordonwoodhull
Copy link
Contributor

Adding (imperfect) support for pie chart resizing while I'm at it.

gordonwoodhull added a commit that referenced this issue Aug 5, 2015
for #544
plus artifacts for last few commits
@gordonwoodhull
Copy link
Contributor

@amergin reports:

Also, it seems that the "set width/height -> redraw()" technique does not work for heatmaps, x axis is not changed properly:

image

The same problem exists for rowcharts as well

@gordonwoodhull
Copy link
Contributor

Each chart just has to be tested, since a lot of attributes are only applied when the elements are created. It's pretty easy to fix the charts with the right testing framework, and that's the main contribution today. If anyone wants to port more test cases from the examples directory to the resizing directory, that would help.

@amergin
Copy link

amergin commented Sep 19, 2015

Not sure if you are aware of this, but the x axis resize problem still exists in beta.19 in the case where the chart is a compositeChart consisting of barCharts. beta.19 seems to have solved the x axis resize problem for rowChart though, which is good.

@gordonwoodhull
Copy link
Contributor

Thanks for the report, @amergin. I was not aware of this. This issue won't be closed until every chart is certifiably fixed, so it may go past 2.0.

@gordonwoodhull
Copy link
Contributor

@dsushmasagar, sorry but the issue tracker is for reporting bugs and making enhancement requests, not for general support questions. Please ask on Stack Overflow with the dc.js tag, or on the user group, and we will be glad to help you there.

I know what you're asking is related to this issue, but it doesn't require any changes to dc.js and it's not going to help this issue along.

@ruhley
Copy link
Contributor

ruhley commented Nov 16, 2015

@gordonwoodhull I have made a lot of progress on responsive charts in my fork. There have been some big changes and some things don't quite work fully yet.

http://ruhley.github.io/dc.js/examples/label-wrap.html

The main responsive changes are:

  • The svg always fits into 100% of its container. The width() and height() methods now set the width and height of the container, but they are not needed.
  • Axis labels, tick labels and legend labels now wrap and ellipses when required.
  • The margin mixin no longer allows the user to set the margins. The margins are automatically calculated to fit in the axis labels, tick labels and legend.

There are heaps of other changes done as well. See them in the changelog.

@gordonwoodhull
Copy link
Contributor

That's great, @ruhley! I hope we are not making conflicting changes. I guess I have mostly been focusing on making non-breaking changes for 2.0, sometimes successfully, sometimes not. If you think the interface needs to be broken in order to fix the functionality, those would be great to consider for 2.1.

(I'm following a weaker form of semantic versioning where the point releases can change the interface but not the point releases, because the interface has such a huge surface area.)

@ruhley
Copy link
Contributor

ruhley commented Nov 17, 2015

My work has recently started getting heavily involved in charts and has authorised me to spend a fair bit of time fixing up dc.js whenever it is needed. The label wrapping and fitting them in properly was the major issue they wanted fixed.

#943 was my original pull request for Paired Row Charts. I have kept my fork up to date and have resolved conflicts myself (don't want to miss out on any fixes you put in!)

@gordonwoodhull
Copy link
Contributor

That's awesome. There is definitely plenty to be fixed. Let's try to get some of your fixes integrated back into dc-js, although it looks like it may be difficult since you have been changing interfaces and not writing tests. I wish I could put more time into this - at least my other projects are also open source vis.

@ruhley
Copy link
Contributor

ruhley commented Nov 19, 2015

Things have been changing quite a lot on my fork recently, so I was going to wait for it to slow down a bit before updating all the tests.

Also my main testing page has been http://ruhley.github.io/dc.js/examples/label-wrap.html, so only the charts on that page have been converted over. I have yet to fix up bubble, choropleth, scatter plot, heat map and box plot.

Hopefully I can get a solid block of time at work to get all of that done...

@ruhley
Copy link
Contributor

ruhley commented Nov 19, 2015

P.S. Also see the new awesome tooltips at http://ruhley.github.io/dc.js/examples/label-wrap.html. Have a look at the pie or row chart to see the border changing colour!

@DilipRajkumar
Copy link

@ruhley tooltip looks really cool..(y)

@gordonwoodhull
Copy link
Contributor

Cool tooltips!

It's really useful to have the code around, whether or not you have time to integrate it yourself. We can pull in stuff on demand.

@nitinsurana
Copy link

+1

@tttp
Copy link
Contributor

tttp commented Apr 17, 2017

Hi,
I discovered today the width(0) trick. Best hidden gem of dc of the day ;)

I'm trying to mix width(0) and rescale() and, for what I understand, it does not mix, because width(0) is generated once.

The workaround I found is to add:

 d3.select(window).on('resize.updatedc', function() {
  dc.events.trigger(function() {
    var graph = graphs.date;
    graph.width(d3.select(graph.anchor()).node().parentNode.getBoundingClientRect().width-graph.margins().right).rescale().redraw();
  },500);
});


@tttp
Copy link
Contributor

tttp commented Apr 17, 2017

@gordonwoodhull I'm trying to make it generic and so far works ok:

d3.select(window).on('resize.updatedc', function() {
  dc.events.trigger(function() {
    dc.chartRegistry.list().forEach(function(chart) {
            var container = chart.root().node();
            if (!container) return; // some graphs don't have a node (?!)
            container=container.parentNode.getBoundingClientRect();
            chart.width(container.width);
            chart.rescale && chart.rescale(); // some graphs don't have a rescale
      });

      dc.redrawAll(); 
  },500);

What about wrapping that into a new dc.responsive(true) ?

@gordonwoodhull
Copy link
Contributor

@tttp, interesting idea! My impression has been that each application has its own way of dealing with responsiveness, but maybe it would be good to just decided on one supported way of doing this.

A couple of notes on your code above:

  • need to deal with multiple chart groups somehow. chartRegistry.list() will only return the charts from the default chart group, which is the common case, but we should support less common cases too.
  • also need to set chart.height(), right?

That's weird that some charts would not have a root node. Any pattern there?

@tttp
Copy link
Contributor

tttp commented Apr 17, 2017

@gordonwoodhull, well, if you'd want to have your own responsiveness, you'd just responsive (false) and do it your own way... but agree, we should offer "the dc way responsiveness" that works 90% out of the box.

  • how do you get all the groups?
  • for what I've seen, height can't be expressed as a function of width, can it? so far, what I did was fix the height and let the width adjust. Worked for what I used as responsive (line+row+pie). Not sure how other graph type behave.
  • the non root. Not sure, will investigate, but does not hurt to be cautious on something that is ran on a window event ;)

(brainstorm mode):

one feature that would make it better is to be able to adjust the ticks, eg if you are at >1000px, you'd put xAxis().ticks(10), if you are in a smaller window, you'd ticks(3)

Not sure if it can work on the on("renderlet") or if it needs a on("resize") event, will try to make more of my viz responsive, see how it goes

@gordonwoodhull
Copy link
Contributor

I suppose we'd have to add .chartGroups() to the chart registry to retrieve the group names. It's currently scoped out of reach on the _chartMap object. Or we'd make the feature somehow tied to a chart group instead of totally global. (Not sure how to do that.)

Different people have different requirements about what should be responsive - there are applications where the height should be responsive too. I think the height could be read from the container as well.

Changing ticks would have to be before any drawing is done, so either the existingpreRedraw event, or a special resize event.

@tttp
Copy link
Contributor

tttp commented Apr 17, 2017

So you'd have to add a new signature on the height

  • height(function(width){ return 42;}), and be sure it's called after width and on the resize event before the rescale/redraw?

My experience with responsive design is based on bootstrap, where the height is usually "whatever it needs to be to display the content once the width is set. For now, fixed height works fine for me, but some stuff (eg. pie chart, heatmaps or choropleth) might benefit being able to adjust the height to keep the ratio constant indeed.

and assuming responsive() is part of dc, it could access _charMap, isn't it? This being said, .chartGroups sounds handy anyway...

I do not know how to test the resize event, is this something that is already covered on your resize examples or does it need to be done by hand?

X+

@gordonwoodhull
Copy link
Contributor

I'm not sure I understand you about the height signature. I just meant, set the height to container.height when we're setting the width to container.width. This is what the resizing examples do currently (although they are using window width and height instead of container - maybe the same in this case).

It might make sense to just add an option to dc.responsive whether responsive height is wanted. Simple.

Unfortunately it can't access _charMap because that's inside the chartRegistry and it would be weird for dc.responsive to live there. But as you said chartGroups should be helpful anyway.

I'd test this by removing the onresize handlers from those resize examples, and using dc.responsive(true) instead. There are no automated tests, I just use those examples as eyeball tests.

@gordonwoodhull
Copy link
Contributor

@tttp, obviously a partial solution will be useful to a lot of people. If you made any progress with this, please go ahead and file a PR.

I'm just noting all the stuff that would have to go into a general-purpose library solution before it could be merged.

(Or if the above snippet is as far as you got, that's already helpful. 😺)

@atomless
Copy link

atomless commented Jun 10, 2017

My prefered solution for responsive sizing is to set a viewBox on the chart svg element and remove the width and height attributes. Then by applying a responsive percentage based width to the chart using css the whole chart resizes and maintains aspect ratio without need for any resize listeners or js resizing. My one remaining problem is that then clicking on the chart triggered a redraw and messes things up. Is it possible to turn disable js resizing and setting of the width and height attributes altogether?

@gordonwoodhull
Copy link
Contributor

Hi @atomless, personally I don't like viewBox approach because the fonts get stretched, but I agree we should support that.

It sounds like you basically want to fix the width and height but not have them reflected in the SVG tag attributes, is that correct?

Could you try commenting out the sizeSvg() call from the top of baseMixin.redraw() and see if that works for you?

@atomless
Copy link

Thanks @gordonwoodhull - I'll give that a try. I do not see any stretching of the fonts with the viewBox method. Everything is just beautifully responsive. I can literally resize the window down tiny or go full screen and chart and text resize perfectly in proportion.

@atomless
Copy link

atomless commented Jun 12, 2017

@gordonwoodhull yes commenting out sizeSvg() in the baseMixin.redraw function does the trick.
Currently I call this function after calling render on a dc chart instance:

var makeResponsiveChart = function(chart_el_id, w, h) {

    $(chart_el_id + ' > svg').removeAttr('height').removeAttr('width').attr('viewBox', '0 0 ' + w + ' ' + h);
};

Where w, and h describe the desired aspect ratio of the chart. Using css the svg element is then set to 100% of it's containing div so it resizes automatically with the browser window. I see no font stretching.

Would be excellent to have support for responsive charts sized with css internally without having to apply these hacks.

@gordonwoodhull
Copy link
Contributor

That's great - I'm glad it works for you.

I'm referring to the text sizing along with the chart - I prefer to have the text a certain size (defined in CSS) and then when the chart is larger the text takes up less room.

But I agree we should also support the viewBox method of sizing, especially because it is simple and works in all cases. I think we should regard it as a different feature.

If you're willing to put these changes under a flag (say chart.viewBoxResizing() or something) and create a PR, that would be great. Otherwise I'll start a new issue and move this conversation there.

@atomless
Copy link

Great. Thanks @gordonwoodhull, I'll make a fork and submit a PR.

@atomless
Copy link

atomless commented Jun 14, 2017

responsiveResize option #1312

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

No branches or pull requests