lolmaus/ember-drag-sort

Inform the developer that they've missed `draggable="true"` on the drag handle

pauln opened this issue · 1 comments

pauln commented

I've just spent more time than I'd care to admit debugging an issue which I initially thought was caused by attempting to use ember-drag-sort within an ember-paper dialog, as I'd been happily using it outside the dialog, but it didn't want to work at all within it. It turned out that I'd missed draggable="true" on my drag handle - but there was no indication that this was the case (other than nothing being draggable).

It would be ideal if either:

  • The drag handle didn't require draggable="true" (though I'm not sure how feasible that would be);
  • The drag handle automatically had draggable="true" set on it if missing;
  • ember-drag-sort raised some sort of warning (at least in dev) if a handle selector is provided but the handle doesn't have draggable="true"; or
  • The handle selector was ignored if the handle didn't have draggable="true", perhaps by adding an attribute selector to the handle selector in the drag-sort-item's $handle computed property and using $handle instead of handle in the draggable computed property.

If it'll help, I can look at submitting a PR - just let me know what the preferred approach is.

Hi! Thank you for your interest in the addon and your desire to contribute.

I agree that there is room for improvement.

  • The drag handle didn't require draggable="true" (though I'm not sure how feasible that would be);

We're relying on HTML5 drag'n'drop capabilities, so the attribute is a requirement.

  • The handle selector was ignored if the handle didn't have draggable="true"

Sounds like moving the source of confusion to another place rather than removing it. :)

  • The drag handle automatically had draggable="true" set on it if missing
  • ember-drag-sort raised some sort of warning (at least in dev) if a handle selector is provided but the handle doesn't have draggable="true"

I'm in favor of either of these options, but there's a catch.

Currently the user is able to control draggable:

{{#drag-sort-list
  items           = myArray
  dragEndAction   = (action 'dragEnd')
  draggingEnabled = isDragonSleeping
  handle          = ".my-handle"
  as |item|
}}
  <span class="my-handle" draggable={{isDragonSleeping}}></span>
  {{item.name}}
{{/drag-sort-list}}

If we stop relying on the user setting the draggable attribute manually, we'll have to bind the value.

I can see two options for this:

  1. Use observers & jQuery. Ugly implementation, but transparent for the user.
  2. Provide a bound component. Clean implementation, but more elaborate for the user. It pays off because we'll no longer need to provide the handle argument maunally:
{{#drag-sort-list
  items           = myArray
  dragEndAction   = (action 'dragEnd')
  draggingEnabled = isDragonSleeping
  as |item handle|
}}
  {{component handle tagName="span"}}
  {{item.name}}
{{/drag-sort-list}}

What do you think about the latter?