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

Line chart doesn't render between points if one falls outside the x-range/focus #949

Open
onorinbejasus opened this issue Jun 11, 2015 · 7 comments
Labels
Milestone

Comments

@onorinbejasus
Copy link

I couldn't find a previous issue that detailed this:

If a point lies beyond the X-axis' current focus/domain, the line between the points will not render. This seems to have been a problem since 2.0.0 alpha 1.

Here is the expected behavior using 1.7.3:
https://jsfiddle.net/7qre8p51/2/
1 7 3

And the current behavior with 2.0.0b12

https://jsfiddle.net/7qre8p51/8/

2 0 0b12

@onorinbejasus
Copy link
Author

The problem stems from the function that domainFilter() uses to compare values to be plotted. As the it stands currently, the comparator is determined by the chart's domain: _chart.x().domain. Thus, data outside of the chart's range will never be rendered -- or drawn to in this case.

The (temporary) solution:

Extend xDomain in domainFilter() to include the previous and next values if they exist:

 var xDomain = _chart.x().domain(),
       prev = _chart.group().top(Infinity).filter(function(d) {return d.key < xDomain[0] } ),
       prev = (prev.length > 0) ? prev[0].key : xDomain[0],
       next = _chart.group().top(Infinity).filter(function(d) {return d.key > xDomain[1] } ),
       next = (next.length > 0) ? next[next.length-1].key : xDomain[1];

       // set the new XDomain
       xDomain = [prev, next]; 

@gordonwoodhull
Copy link
Contributor

Thanks @onorinbejasus, that sounds about right.

@gordonwoodhull gordonwoodhull added this to the v2.0 milestone Jun 11, 2015
@onorinbejasus
Copy link
Author

I actually like this solution more, but both work. The benefit of this one is that we don't have to worry about filters that may exist on the crossfilter object:

        dc.stackMixin = function (_chart) {
            function prepareValues (layer, layerIdx) {
                var valAccessor = layer.accessor || _chart.valueAccessor();
                layer.name = String(layer.name || layerIdx);
                layer.values = layer.group.all().map(function (d, i) {
                    return {
                        x: _chart.keyAccessor()(d, i),
                        y: layer.hidden ? null : valAccessor(d, i),
                        data: d,
                        layer: layer.name,
                        hidden: layer.hidden
                    };
                });

                layer.values = layer.values.filter(domainFilter(layer.values));
                return layer.values;
            }

            var _stackLayout = d3.layout.stack()
                .values(prepareValues);

            var _stack = [];
            var _titles = {};

            var _hidableStacks = false;

            function domainFilter(values) {
                if (!_chart.x()) {
                    return d3.functor(true);
                }

                var xDomain = _chart.x().domain(),
                      minX = xDomain[0],
                      maxX = xDomain[1];    

                if (_chart.isOrdinal()) {
                    // TODO #416
                    //var domainSet = d3.set(xDomain);
                    return function () {
                        return true; //domainSet.has(p.x);
                    };
                }
                if (_chart.elasticX()) {
                    return function () { return true; };
                }

                if (values) {
                  maxX = d3.min(values, function(d) {
                    return d.x < xDomain[1] ? Math.MAX_VALUE : d.x;
                  });

                  minX = d3.max(values, function(d) {
                    return d.x > xDomain[0] ? Math.MIN_VALUE : d.x;
                  });
                }

                return function (p) {
                    return p.x >= minX && p.x <= maxX;
                };
            }

The major changes here are

  1. passing the values array into the domainFilter function:
layer.values = layer.values.filter(domainFilter(layer.values)); 

...

 function domainFilter(values) { ...
  1. evaluating the bounds if the values array exists:
 if (values) {
   maxX = d3.min(values, function(d) {
       return d.x < xDomain[1] ? Math.MAX_VALUE : d.x;
     });

     minX = d3.max(values, function(d) {
           return d.x > xDomain[0] ? Math.MIN_VALUE : d.x;
     });
 }
  1. and returning the new evaluation function:
return function (p) {
   return p.x >= minX && p.x <= maxX;
};

@mtraynham
Copy link
Contributor

Tagging #525, somewhat related.

@gordonwoodhull
Copy link
Contributor

Root cause is also related to #507 - need to include a domain larger than the chart.

@gordonwoodhull
Copy link
Contributor

This SO question is probably a manifestation: http://stackoverflow.com/q/37033997/676195

gordonwoodhull added a commit that referenced this issue Jun 10, 2016
gordonwoodhull added a commit that referenced this issue Jun 11, 2016
gordonwoodhull added a commit that referenced this issue Aug 4, 2016
to display the off-edge bug #949
gordonwoodhull added a commit that referenced this issue Sep 12, 2016
to display the off-edge bug #949
gordonwoodhull added a commit that referenced this issue Jan 16, 2017
composite 2-pass drawing pre- & post- scale change

coord.plotData shall returns continuation if it wants to participate
the immediate action draws anything before the transition
the continuation will be run after the scale change

note there is nothing asynchronous here. instead we are using
continuations for the sake of capturing the joined selections that both
passes need.

2-pass is implemented but not specialized for the bar chart only

this commit also makes explicit that axis preparation and scale-setting
is different for render and redraw. render sets the scales twice,
probably to the same thing. this is because the scales must be valid
for the first pass, and they must *not* be set before a redraw, because
they must be consistent with the old view.

we'll see if this causes problems when clients (& maybe focus chart)
explicitly change the x domain. but if there are problems here, they are
probably not new. any enter selections on a too-soon scale change were
probably already screwed up.

implement 2-phase drawing for bar chart

but, as i guessed might happen, this does not work for cases where
the scale was updated by the client

copy/set/restore scales to make last available

we must let the user change the scale, so we will need to copy
after each draw. however, this push/pop stuff is probably wrong
(and 2-phase is probably wrong too).

will clean up later after i have a working poc.

apply barWidth to entering

this logic is just wrong

what more is there to say

the intent here was not to string-compare null
but it ended up skipping zeroes

this is still somewhat wrong because it *should* string-compare zeroes
but let's just deal with the obvious and move on

fetch data before restoring scales

... of course, the idea that the data should depend on the current
scale is infuriating in itself, but again, this branch is a poc

test left-to-right swipe

currently the middle is lost

make state explicit

it's a lot of parameters but it seems much more clear to pass them
in rather than assuming they will be set on the chart.

get rid of the very clever continuation trick i just put in. :-|

also stop caching barWidth and xUnitCount, which wouldn't work
anymore (since we have to look at the past and present at once).

this works but a lot of tests are failing

it's now possible to observe scale changing type

from linear to ordinal, for example
fun.

also track last right Y scale

and don't pass the left :)

this was not the way to process data better

we do want to call .data() less, but this didn't actually help
we do want to get data for the right range, this didn't do that either

this was only here to get data at the right scale-moment
but we're past that now

fade out + refinements for ordinal bar transitions

always fade out as we're fading in

ordinal bars should not move or size as they enter/exit, because
logically there is nowhere for them to come from / go to.

pull domainFilter out of prepareValues

too deep, too stateful

start transitioning from .data() to computeStacks

bar chart draw bounds enclosing old and new domain

to avoid the gap

technically we could remove what's outside the drawing area afterward

rotate through a few more spans

to display the off-edge bug #949
gordonwoodhull added a commit that referenced this issue Jun 23, 2017
@gordonwoodhull
Copy link
Contributor

Until this is fixed properly, there is a flag evadeDomainFilter which just skips filtering of points. It's available in dc.js 2.0.4 and 2.1.7.

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

No branches or pull requests

3 participants