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

SortableList handleSelector bug #430

Closed
orestis opened this Issue Feb 26, 2015 · 6 comments

Comments

Projects
None yet
3 participants
@orestis

orestis commented Feb 26, 2015

Hello,

It seems like the code for handleSelector only works if the selector you pick is the deepest one. For example:

<ul class="ink-sortable-list" data-handle-selector=".drag-handle">
    <li><div class="drag-handle"><img src="someimage.jpg"></div>Content 1</li>
    <li><div class="drag-handle"><img src="someimage.jpg"></div>Content 2</li>
    <li><div class="drag-handle"><img src="someimage.jpg"></div>Content 2</li>
</ul>

The above code would fail, whereas specifying data-handle-selector="img" would work.

It seems the issue is this snippet::

        if(this._options.handleSelector && !Selector.matchesSelector(ev.target, this._options.handleSelector)) { return; }

The target of the event might be deeper than our selector. I'm not familiar with Inks internals, but it doesn't seem this traverses up the tree to locate the selector.

@fabiosantoscode

This comment has been minimized.

Show comment
Hide comment
@fabiosantoscode

fabiosantoscode Feb 26, 2015

Contributor

I noticed that last week as I wrote Ink docs and it's on my to-do list for Ink.

As a workaround, you can select your deepest element (the image in this case) for now.

Contributor

fabiosantoscode commented Feb 26, 2015

I noticed that last week as I wrote Ink docs and it's on my to-do list for Ink.

As a workaround, you can select your deepest element (the image in this case) for now.

@orestis

This comment has been minimized.

Show comment
Hide comment
@orestis

orestis Feb 26, 2015

Thanks. Yes, I'm using that workaround now (actually doing ".drag-handle img" to be more specific). Perhaps related, I need to set draggable=false to the image because the browser would try to browse that out. Is there a way for Ink to do this automatically ?

orestis commented Feb 26, 2015

Thanks. Yes, I'm using that workaround now (actually doing ".drag-handle img" to be more specific). Perhaps related, I need to set draggable=false to the image because the browser would try to browse that out. Is there a way for Ink to do this automatically ?

@fabiosantoscode

This comment has been minimized.

Show comment
Hide comment
@fabiosantoscode

fabiosantoscode Feb 26, 2015

Contributor

I don't think it's in scope for the sortable list to solve that, nor is it very useful. That's behaviour specific for images and links, and drag handles are mostly icons.

Contributor

fabiosantoscode commented Feb 26, 2015

I don't think it's in scope for the sortable list to solve that, nor is it very useful. That's behaviour specific for images and links, and drag handles are mostly icons.

@mariomc

This comment has been minimized.

Show comment
Hide comment
@mariomc

mariomc Feb 26, 2015

Contributor

If you want, you can try to disable dragging with CSS having a selector similar to this:

.drag-handle img {
  -webkit-user-select: none;
  -khtml-user-select: none;
  -moz-user-select: none;
  -o-user-select: none;
  user-select: none;
}

Granted, it won't work in some older browsers but depending on what you want to accomplish, might be worth the shot.

Contributor

mariomc commented Feb 26, 2015

If you want, you can try to disable dragging with CSS having a selector similar to this:

.drag-handle img {
  -webkit-user-select: none;
  -khtml-user-select: none;
  -moz-user-select: none;
  -o-user-select: none;
  user-select: none;
}

Granted, it won't work in some older browsers but depending on what you want to accomplish, might be worth the shot.

@fabiosantoscode

This comment has been minimized.

Show comment
Hide comment
@fabiosantoscode

fabiosantoscode Mar 13, 2015

Contributor

Status update: @suskind has worked and will submit a bunch of improvements for SortableList, and fixing this (the original issue) is postponed to avoid merge conflicts. Give it a week or so.

Contributor

fabiosantoscode commented Mar 13, 2015

Status update: @suskind has worked and will submit a bunch of improvements for SortableList, and fixing this (the original issue) is postponed to avoid merge conflicts. Give it a week or so.

@fabiosantoscode

This comment has been minimized.

Show comment
Hide comment
@fabiosantoscode

fabiosantoscode Mar 26, 2015

Contributor

Thanks for reporting this. Closed in f164318

Contributor

fabiosantoscode commented Mar 26, 2015

Thanks for reporting this. Closed in f164318

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