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

Restore preventDefault, but use tabindex to focus the parent element. #1326

Closed
wants to merge 1 commit into from

Conversation

mbostock
Copy link
Member

Work-in-progress. Supersedes #1322. Should fix #1321 #1288 #1099.

Tasks:

  • Applying the drag behavior, zoom behavior or brush should set the tabindex attribute and outline style on the nearest HTML ancestor element (the ownerSVGElement when used within SVG, and the current element when used within HTML).
  • If the tabindex attribute is already set, it should be preserved? What about the outline style? It might be safe to ignore this possibility.
  • The mousedown event should be cancelled (using preventDefault and stopPropagation).
  • The touchstart event should not be cancelled for the drag behavior, as this disables draggable links. (Canceling mousedown does not prevent click, but canceling touchstart effectively does.)

This is a partial revert of 53cd4d4. This will
be followed by using the tabindex attribute to focus a parent element.
@jasondavies
Copy link
Contributor

We should also only bind mouse events for brush on mousedown (not on touchstart), like in #1322. I checked and the same problem happens with this branch (with preventDefault turned back on). I can put it in a separate request if you prefer!

@mbostock
Copy link
Member Author

Feel free to push commits to this branch. But I’m still ruminating over @jfirebaugh’s comments and haven’t decided yet whether I agree. However I am weighing his arguments against the precedent set by other libraries, which seem to favor preventDefault. (Not that we have to agree with every other library, but it’s empirical evidence to consider at least.)

@mbostock mbostock closed this Jun 25, 2013
@mbostock
Copy link
Member Author

Closing in favor of preventDefault approach.

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.

2 participants