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

Allow optional drag event enabling #2579

Closed
wants to merge 1 commit into from

Conversation

alecgibson
Copy link
Contributor

This change allows consumers to enable drag events by configuring Quill
with an enableDragging flag.

This is useful for writing custom modules that want to use drag
behaviour.

This change allows consumers to enable drag events by configuring Quill
with an `enableDragging` flag.

This is useful for writing custom modules that want to use drag
behaviour.
@jhchen
Copy link
Member

jhchen commented Apr 16, 2019

Preventing dragging in Quill was motivated by some browsers that allow images and other elements (iirc images in Firefox, links in Edge) to be moved natively but such actions would mess up the DOM structure that Quill expects and tries to keep tidy and predictable.

What is your use case for dragstart?

@alecgibson
Copy link
Contributor Author

Our use case is allowing dragging of certain custom blots around the DOM. For example, we have a heavily-customised image block blot, which has an associated caption attached to it. Manipulating the image (moving / deleting) will also make sure we correctly move / delete the associated caption.

This seems like a much more user-friendly approach to the alternatives, which are:

  • highlight (carefully including the whole caption), cut and paste
  • delete the whole thing, re-upload, and retype the caption
  • cut and paste text around the image (people just don't think like that)
  • add other UI elements like arrows to move the image up / down

Apr-17-2019 07-49-34

@alecgibson
Copy link
Contributor Author

@jhchen if you don't want to add this flag, can we please at least move the listener registration into a module like the uploader so that people can at least override that behaviour if they want?

@jhchen
Copy link
Member

jhchen commented Apr 25, 2019

I don't want to add the flag but not sure how to best let people override it. This guard doesn't really make sense in the uploader as its purpose is unrelated to the uploader. Somewhere in ScrollBlot might be more appropriate.

@alecgibson
Copy link
Contributor Author

@jhchen if you want it in the Scroll, then perhaps putting it in a method would let people extend and override the behaviour? eg:

class Scroll extends ScrollBlot {
  constructor(registry, domNode, { emitter }) {
    // Do some constructory stuff
    this.domNode.addEventListener('dragstart', e => this.preventDrag(e));
  }

  // Users can now extend this class and override this method
  preventDrag(e) {
    e.preventDefault();
  }
}

We could also do the above on the Quill class, although extending that class feels a little bit icky.

If you're happy with the above proposal, let me know and I'll put together a new PR.

@jhchen
Copy link
Member

jhchen commented Apr 25, 2019

Yes that works although can you name it handleDrag or handleDragStart? Thanks!

@alecgibson
Copy link
Contributor Author

Superseded by #2596

@alecgibson alecgibson closed this Apr 26, 2019
@alecgibson alecgibson deleted the drag-and-drop branch April 26, 2019 07:16
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.

2 participants