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

drag.on("init") ? #67

Closed
Fil opened this issue Jan 18, 2020 · 7 comments
Closed

drag.on("init") ? #67

Fil opened this issue Jan 18, 2020 · 7 comments

Comments

@Fil
Copy link
Member

Fil commented Jan 18, 2020

When calling drag(selection) I'd like to be able to add a className or a style to draggable elements.

Currently I have to do it outside the dragging function:

const drag = d3.drag()…;

d3.selectAll("circle")
   .style("cursor", "grab")
   .call(drag)

It would be nice in terms of separation of concerns to be able to define it in the drag behaviour itself (which can set cursor:grabbing during effective dragging).

const drag = d3.drag()
     .on("init", (selection) => selection.style("cursor", "grab"))…;

d3.selectAll("circle")
   .call(drag)

This could be done in https://github.com/d3/d3-drag/blob/master/src/drag.js#L40 with a default init = identity.

@Fil
Copy link
Member Author

Fil commented Jun 11, 2020

Another use case (with d3-zoom but it's exactly the same idea), is to help the initialization of the target's zoom factor, and initial draw, in https://observablehq.com/@fil/synchronized-projections

@mbostock
Copy link
Member

And the init event would be dispatched in here?

d3-drag/src/drag.js

Lines 39 to 48 in 652167e

function drag(selection) {
selection
.on("mousedown.drag", mousedowned)
.filter(touchable)
.on("touchstart.drag", touchstarted)
.on("touchmove.drag", touchmoved)
.on("touchend.drag touchcancel.drag", touchended)
.style("touch-action", "none")
.style("-webkit-tap-highlight-color", "rgba(0,0,0,0)");
}

Fil added a commit that referenced this issue Jun 16, 2020
@Fil Fil mentioned this issue Jun 16, 2020
@Fil
Copy link
Member Author

Fil commented Jun 16, 2020

@Fil
Copy link
Member Author

Fil commented Jun 16, 2020

And another test to verify that we can bind all listeners (start drag end) on init.
https://observablehq.com/d/1999aec3a613880e

@mbostock
Copy link
Member

In this notebook, we implement initialization behavior (setting the __zoom property) by wrapping the zoom behavior in a function:

https://observablehq.com/@d3/versor-zooming

The awkward part is that when wrapping you must re-expose zoom.on so that callers can register event listeners.

While an init event would make this easier, I’m not sure we want to head in this direction. It feels more like inheritance than composition: we’re dependent on the behavior to expose all the events we need to implement our higher-level behavior. For example, what if you want to remove the styles when the element is no longer draggable, would we need a destroy event? (And should the events be called bind and unbind?) And would the drag behavior then need a drag.remove function instead of using selection.on to remove the drag listeners?

This discussion reminded me of an idea I’ve had for a while but apparently never wrote down: adopting native events. #73 That would also make it easier to build higher-level behaviors because you wouldn’t have the awkward aspect of re-exposing zoom.on (or drag.on): listeners are applied to elements rather than behaviors. So instead of:

  const zoom = d3.zoom()
      .scaleExtent(scaleExtent.map(x => x * scale))
      .on("start", zoomstarted)
      .on("zoom", zoomed);

  

  return Object.assign(selection => selection
      .property("__zoom", d3.zoomIdentity.scale(projection.scale()))
      .call(zoom), {
    on(type, ...options) {
      return options.length
          ? (zoom.on(type, ...options), this)
          : zoom.on(type);
    }
  });

You might write:

  const zoom = d3.zoom()
      .scaleExtent(scaleExtent.map(x => x * scale));

  

  return selection => selection
      .property("__zoom", d3.zoomIdentity.scale(projection.scale()))
      .on("zoomstart", zoomstarted)
      .on("zoom", zoomed)
      .call(zoom);

@Fil
Copy link
Member Author

Fil commented Jun 17, 2020

The awkward part is that when wrapping you must re-expose zoom.on

yes I had to read that code 10 times to get a sense of how it was constructed :) But this is indeed what I want to facilitate with this "init" event proposal. Maybe trough another technical approach though. https://observablehq.com/compare/1ea380bf05fbf68c@316...c189fc97835332a7@319 works well (though I don't like having to set up the cursor at two very different places).

@Fil
Copy link
Member Author

Fil commented Jun 24, 2020

Another way of doing some "initial" work with the current code base is by diverting .touchable() :

  const touchable = zoom.touchable();

  zoom
    .touchable(function() {
      d3.select(this)
          .property("__zoom", d3.zoomIdentity.scale(projection.scale()))
          .style("cursor", "grab");
      return touchable.apply(this, arguments);
    })
    .on("start.cursor zoom.cursor end.cursor", function() {
      d3.select(this).style("cursor", d3.event.type === "end" ? "grab" : "grabbing");
    });

@Fil Fil closed this as completed Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants