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

Generalize suppression of undesired default behaviors when dragging. #1341

Closed
wants to merge 2 commits into from

Conversation

mbostock
Copy link
Member

Fixes #1321.

The new d3_event_dragSuppress method serves a similar purpose to the previous
d3_event_userSelect, except it generalizes to prevent all undesired browser
default behaviors when dragging. In addition to text selection, this includes
dragging (of links and images) and clicks (on links). Suppressing click events
is optional and can be controlled by the caller.
@tpreusse
Copy link
Contributor

tpreusse commented Jul 7, 2013

this change broke common, documented usage of force.drag

node
    .on('click', function(d) {
        // do something
    })
    .call(force.drag);

now needs to be:

node
    .on('touchstart', function(d) {
        d3.event.preventDefault(); // no scrolling
    })
    .on('click', function(d) {
        if(!d3.event.defaultPrevented) {
            // do something
        }
    })
    .call(force.drag);

However I do like the change. Not sure about the right approach to patching this: patch layout/force.js (at least touchstart would seem reasonable) and or create some awareness for the breaking change - update docu, examples etc.

@mbostock
Copy link
Member Author

mbostock commented Jul 7, 2013

I don’t think you need the d3.event.defaultPrevented check; it’s still the case that the drag behavior will suppress clicks when the node is dragged.

But you are correct that we no longer preventDefault on touchstart… We decided we didn’t want to disable the browser behaviors by default, since it’s impossible to undo the effect of preventing the default behavior, but it’s always possible to prevent it in your own code. On the other hand, we do prevent selectstart and dragstart events, so I do wonder whether there’s a way to disable scrolling here.

Ugh, browser events.

@tpreusse
Copy link
Contributor

tpreusse commented Jul 7, 2013

!d3.event.defaultPrevented is needed because default gets prevented but propagation still happens - my click listener gets called with a default prevented event.

@mbostock
Copy link
Member Author

mbostock commented Jul 7, 2013

Ah, you’re right. We just preventDefault on that click, not stopPropagation.

@tpreusse
Copy link
Contributor

tpreusse commented Jul 7, 2013

yeah it's quite tricky but checking for d3.event.defaultPrevented is not a major hassle and gives you full control - just needs to be documented (if desired). Btw.: I upgraded specifically to get the touchend with force.drag for a tooltip - which is nice and was really hard before.

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

Successfully merging this pull request may close these issues.

None yet

2 participants