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

Added the ability to pass an element as the handle #56

Closed
wants to merge 1 commit into from

Conversation

beauwest
Copy link

Added the ability to pass an element as the handle. This enables Polymer ShadowDom elements to be used as the handle as well. Or just passing an element directly if you don't want it to use querySelectorAll();

Added the ability to pass an element as the handle. This enables
Polymer ShadowDom elements to be used as the handle as well.
@desandro
Copy link
Owner

Thank you for this contribution! This could be a useful feature. But I'd prefer to hear other's input before it gets merged.

If you would like to see this feature please +1

@beauwest
Copy link
Author

Do I get to 👍 my own? Haha!

Just to clarify, this will not break any existing selectors. All it does is extend the handle: configuration to allow passing a reference to a DOM element that you already have access to. Theoretically it could make getting the handler a tiny bit more efficient if you already have the DOM Element reference defined.

@beauwest
Copy link
Author

One thing to note, Draggabilly already does this on the element that's being passed in:

this.element = typeof element === 'string' ? document.querySelector( element ) : element;

@beauwest
Copy link
Author

Is there any update on this? Thanks @desandro.

@davitv
Copy link

davitv commented Jan 29, 2015

+1
Same code for handle i have added for my own project when was implementing tree with drag'n'drop functions. The problem was that the handle class for a particular draggable tree element existed also in it's child elements and i needed way to define already selected element as a handler. Would be nice to see it in master branch

@desandro
Copy link
Owner

@davitv I recommend using a unique handle class if that's the case.

I'm closing this PR as this feature didn't get enough support.

@desandro desandro closed this Jan 30, 2015
@beauwest
Copy link
Author

It's your project, I'm just very confused why a simple, non-conflicting code change, that in some cases prevents double querySelector/elementByID lookups, would be rejected.

@ahenager
Copy link

I'd also like this change, and it wouldn't hurt anything else, would it? I always try to avoid having to run my own fork when I can, and this change is much preferable to having extra code in my project to deal with this.

@nomadme
Copy link

nomadme commented Jan 30, 2015

+1

3 similar comments
@shepherdsam
Copy link

+1

@navanjr
Copy link

navanjr commented Jan 30, 2015

+1

@gillsonkell
Copy link

+1

@davitv
Copy link

davitv commented Jan 31, 2015

No @desandro, i don't want to create an unique class for every tree node, and why should i, if it is better to pass a handle as a dom element, which is already selected before for other purposes.

@ferllings
Copy link

Any news on this?
I have an Polymer element where the handle is inside the ShadowDOM.
So this would be very useful.

@beauwest
Copy link
Author

beauwest commented Sep 4, 2015

In the latest version, since my pull request was rejected, I've had to overwrite the setHandles method.

var handleElement = this.$.handle;
Draggabilly.prototype.setHandles = function () {
    this.handles = [handleElement];
    this.bindHandles();
};

Not ideal at all, but it's the simplest way to fix the issue that prevents Draggabilly from supporting native ShadowDOM.

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.

None yet

9 participants