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

Pass the node to the supstance drag event listener #98

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

sf-wind
Copy link
Contributor

@sf-wind sf-wind commented Nov 17, 2015

In my use case, when dragging a line, some other related lines also move. I use it to implement fibonacci trend, so the registered drag event listener needs to know which line is dragged. So add the node as a second argument.

Conflicts:
	dist/techan.js
	dist/techan.min.js
	dist/techan.min.js.map

Merge from forked source.
to the tick date, even if that date is smaller than the tick date.
Currently it only uses the closest date that is greater than
the tick date.
@@ -308,7 +308,16 @@ module.exports = function(d3_scale_linear, d3_time, d3_bisect, techan_util_rebin
return function(d) {
var value = visibleDomainLookup[+d];
if (value !== undefined) return visibleDomain[value];
return visibleDomain[d3_bisect(visibleDomain, d)];
var index = d3_bisect(visibleDomain, d);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this pull request #100?

@andredumas
Copy link
Owner

Hi @sf-wind

Thanks again for the contribution.

This appears to have pull request #100 merged in already making a bit harder to review. I also won't be able to merge it in until #100 is merged in. Any chance those commits could be taken out? Otherwise this one will have to wait until then.

cheers
andre

@@ -57,7 +57,7 @@ module.exports = function(d3_behavior_drag, d3_event, d3_select, d3_dispatch, ac
accessor.v(d, value);
annotationSelection.each(plot.annotation.update(annotation, d3_event().y));
refresh(g, plot, accessor, x, y, annotationSelection, annotation);
dispatch.drag(d);
dispatch.drag(d, g);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is handy, but it should align to d3 the api, generally passing (d, i) as parameters and this being the node as documented here https://github.com/mbostock/d3/wiki/Selections#on

The specified listener is invoked in the same manner as other operator functions, being passed the current datum d and index i, with the this context as the current DOM element.

@sf-wind
Copy link
Contributor Author

sf-wind commented Nov 24, 2015

Thanks. This is my first pull request and I did it on my master branch by mistake.

As for the d3 api, I'm not quite familiar with and I will definite take a look.

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

Successfully merging this pull request may close these issues.

None yet

2 participants