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

Prevent click event on drop #405

Open
schorke opened this issue Jan 8, 2021 · 2 comments
Open

Prevent click event on drop #405

schorke opened this issue Jan 8, 2021 · 2 comments

Comments

@schorke
Copy link

schorke commented Jan 8, 2021

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)
@ygongdev
Copy link
Member

ygongdev commented Jan 8, 2021

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.

@MelSumner
Copy link
Contributor

@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?

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

No branches or pull requests

3 participants