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

Widget: Destroy only when element is the actual target #736

Closed
wants to merge 1 commit into from
Closed

Widget: Destroy only when element is the actual target #736

wants to merge 1 commit into from

Conversation

marcandre
Copy link
Contributor

If ever a 'remove' event was not triggered on the actual widget but on a child and is just bubbling up, then nothing should be done.

Use case: I use a 'remove' event (with bubbling) in my app and without this patch it interferes if the element I trigger it on is contained in a jQuery-ui widget.

http://stackoverflow.com/questions/12584732/how-come-sortable-area-freezes-if-a-remove-event-is-triggered

this._on({
remove: function( event ) {
if (event.target === element) {
instance.destroy();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use this.destroy() here --- _on ensures the widget instance is the context of the callback

@gnarf
Copy link
Member

gnarf commented Oct 9, 2012

This seems to make sense to me, @scottgonzalez might have some reason we can't do this. Also, is there a UI ticket on http://bugs.jqueryui.com/ for this? Could you file one? We generally don't accept pulls without a ticket.

@scottgonzalez
Copy link
Member

I'd rather just fix the naming of our events. I'll look into whether we can get that done for 1.10. If we can, I'll just update this to now, if we can't I'll land this PR (without the instance var).

@marcandre
Copy link
Contributor Author

I fixed my pull request.
Created an issue (got that system is slow... please consider using github for that too. and what's up with the silly anti-spam filter?) http://bugs.jqueryui.com/ticket/8652

@scottgonzalez If you can rename the event, that's fine too, but even then this patch wouldn't hurt, would it?

@scottgonzalez
Copy link
Member

@marcandre I'm going to land this. Can you sign our CLA?

@scottgonzalez
Copy link
Member

Landed in 8bb05d2.

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

Successfully merging this pull request may close these issues.

None yet

3 participants