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

[v2 -> v3] ScatterPlot no longer removes symbols #1460

Closed
Frozenlock opened this issue Jul 4, 2018 · 12 comments
Closed

[v2 -> v3] ScatterPlot no longer removes symbols #1460

Frozenlock opened this issue Jul 4, 2018 · 12 comments

Comments

@Frozenlock
Copy link
Contributor

V2 behavior

The scatter plot translates or removes the various symbols when we regroup the dimension.

peek 2018-07-04 18-44


V3 behavior

The scatter translates but does not remove the various symbols when we regroup the dimension.

peek 2018-07-04 18-45

@Frozenlock
Copy link
Contributor Author

The same problem occurs with simple filtering.

V2

We can see symbols appearing/disappearing.
peek 2018-07-04 18-06

V3

Unused symbols are 'stuck' in place.
peek 2018-07-04 18-07

@gordonwoodhull
Copy link
Contributor

I could be wrong but I don't think anything has changed here. I think the scatter plot just has confusing behavior due to the problems described in #621.

Also recall that crossfilter entries don't go away when the value goes to zero. So you have to map either radius or opacity to the value. By default the chart will apply emptySize if the existenceAccessor returns falsy. By default the existenceAccessor returns d.value.

It definitely works in some cases.

http://dc-js.github.io/dc.js/examples/scatter-brushing.html

Happy to help you debug if you share a fiddle or other live example.

@Frozenlock
Copy link
Contributor Author

For a moment I thought it was caused by the last modifications to the scatter source, but I was wrong.
It's really weird how it just stopped working.

Playing around with the existenceAccessor, I'm stating to think that the removed group keys are never retested.

@Frozenlock
Copy link
Contributor Author

The only custom thing I can think iis using reductio to group both the X and Y values.
I'll try to change my configs and post back here if I find something.

@Frozenlock
Copy link
Contributor Author

Frozenlock commented Jul 6, 2018

I think I found something!

When updated, the following snippet does not return fewer objects in V3, but does in V2.

chart.chartBodyG().selectAll('path.symbol')

You can see it used here :
https://github.com/dc-js/dc.js/tree/develop/src#L81


EDIT :
Never mind, I was so happy to find a potential path that I completely overlooked the obvious : of course it doesn't return fewer objects, that's the whole problem to begin with.

@Frozenlock
Copy link
Contributor Author

Found the problem.

From my very quick reading of D3, .merge is to join together the new data with the existing DOM objects. (More info here : d3/d3-selection#86)

This means that we must remove the symbols before merging, otherwise we find ourselves with both the new and old objects.

Simply moving the symbols.exit() before the merging is enough to restore the V2 behavior.

        var symbols = _chart.chartBodyG().selectAll('path.symbol')
            .data(_chart.data());

	
        dc.transition(symbols.exit(), _chart.transitionDuration(), _chart.transitionDelay())
            .attr('opacity', 0).remove();

@kum-deepak
Copy link
Collaborator

@Frozenlock that definitely was incorrect. Will you be creating a pull request or should I create one?

@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented Jul 6, 2018

Aha! You are adding and removing points from the scatter plot. So you must either be using a different backend that's not crossfilter, or using fake groups.

We'll definitely need to start testing this use case.

Don't you also run into trouble with points moving around when an earlier point was removed from the data set, due to #621?

That makes sense about putting exit before merge, as @kum-deepak was careful to do that everywhere else.

(I guess the commit you mentioned earlier was a red herring, as you've deleted your comment. I'll delete my comment since it doesn't make sense to respond to something that's not there. 😉 )

The only thing I'm confused about is whether it's symbols.exit() or .remove() that makes the difference. I thought that the new D3 selection interface was side-effect-free, which would mean we need to remove before merge. But remove is on a delay because of the transition!

So I'm glad this works, but it still doesn't seem safe to me. @kum-deepak, do you understand this better?

@Frozenlock
Copy link
Contributor Author

@kum-deepak I'll leave it to you.

@gordonwoodhull Yes I apologize for my deleted message. I deleted it when I realized it was completely wrong, but you posted an answer just a few seconds after. (Probably due to Github's sync delay.)

You are right, I am using fake groups! In this case the fake group is used with reductio to build the key as the [x y] values. Because the x and y values are averages (or min or max), they change pretty much every time a filter is applied. This is why points moving around is not a problem; they all move around.

@gordonwoodhull
Copy link
Contributor

Got it, thanks - the fake group explains everything. (And yeah, I probably read the email and responded without noticing that you had already deleted your comment.)

What I mean about movement being a problem: this movement doesn't really have meaning, does it?

I mean, it looks cool, but there aren't identifiable entities moving from x0,y0 to x1,y1. It's just one set of points and another set of points; ideally one set would fade out and the other set would fade in, rather than showing movement.

Can't quite see this in the low frame rate of the GIF you posted, but I suspect that a false impression is being conferred... Not that I have any solution for that except to turn off transitions...

@Frozenlock
Copy link
Contributor Author

@gordonwoodhull @kum-deepak You guys are amazing, thank you very much!

@Frozenlock
Copy link
Contributor Author

What I mean about movement being a problem: this movement doesn't really have meaning, does it?

A quick comment on this because I found a way to convey more meaning than the random transition : using a fake group to sort the data greatly reduces the movement.

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

3 participants