ember-sortable icon indicating copy to clipboard operation
ember-sortable copied to clipboard

Prevent click event on drop

Open schorke opened this issue 4 years ago • 2 comments

Describe the bug Click event inside a sortable-item is triggered on drop.

To Reproduce

import Component from '@glimmer/component';
import { action } from '@ember/object';
import { tracked } from '@glimmer/tracking';
import { A } from '@ember/array';

export default class DragList extends Component {
  @tracked
  _items = [ 'Uno', 'Dos', 'Tres', 'Cuatro', 'Cinco' ];

  get items() {
    return this._items;
  }

  @action
  update(newOrder) {
    this._items = A(newOrder);
  }

  @action
  rowClicked() {
    console.log('clicked');
  }
}
<ol {{sortable-group groupName="group1" onChange=this.update}}>
  {{#each this.items as |item|}}
    <div {{on "click" this.rowClicked}}>
      <li {{sortable-item groupName="group1" model=item}} >
        {{item}}
      </li>
    </div>
  {{/each}}
</ol>

=> This will not trigger the rowClicked action, which is the expected behaviour here since there is this code which will not propagate / bubble up the event.

<ol {{sortable-group groupName="group2" onChange=this.update}}>
  {{#each this.items as |item|}}
    <li {{sortable-item groupName="group2" model=item}} >
      <div {{on "click" this.rowClicked}}>
        {{item}}
      </div>
    </li>
  {{/each}}
</ol>

=> This will trigger the rowClicked action, which I don't want to happen. Is this a known / expected behaviour ? Do you have any fix / workaround ?

My workaround is to assert parent's class is-dropping in the click event like this:

@action
rowClicked(e) {
  if (!e.target.parentElement.classList.contains('is-dropping')) {
    console.log('clicked');
  }
}

Expected behavior Click event inside sortable-item should not be triggered when dropping it. We previously used ember-drag-drop which had this behaviour built in.

Other info

  • "ember-sortable": "2.2.1"
  • "ember-cli": "3.20.0"
  • Chrome Version 87.0.4280.141 on MacOS
  • (Using Ember Octane, Glimmer components & ember-sortable modifier)

schorke avatar Jan 08 '21 10:01 schorke

I can take a look, but just 2 things from my side:

  1. From a purely performance standpoint, it seems like using event delegation here would be more ideal, e.g attach click listener on the ol and attach an identifier (e.g data-*) on each row.

  2. if you were to use a handle, it should solve this problem and perhaps provide a better a11y experience as well.

ygongdev avatar Jan 08 '21 18:01 ygongdev

@ygongdev were you able to look at this issue?

Additionally, I wonder if we should implement improved documentation and guidance? It seems like OP should refactor a bit instead of adjusting the addon behavior. WDYT?

MelSumner avatar Feb 07 '22 01:02 MelSumner