Skip to content

Tooltip: Only bind remove handler for delegated tooltips #1156

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

Closed
wants to merge 1 commit into from

Conversation

scottgonzalez
Copy link
Member

The remove handler was being added for every tooltip, but only removed for
delegated tooltips. The default destroy behavior already handles non-delegated
tooltips, so the handler should only be added for delegated tooltips.

Fixes #9531

I'm not sure if there's a reasonable way to test this. We don't generally test for memory leaks.

The remove handler was being added for every tooltip, but only removed for
delegated tooltips. The default destroy behavior already handles non-delegated
tooltips, so the handler should only be added for delegated tooltips.

Fixes #9531
@tjvantoll
Copy link
Member

This looks good to me. Is it worth having an _isDelegated() method to handle the duplicated target[ 0 ] !== this.element[ 0 ] check?

@scottgonzalez
Copy link
Member Author

Perhaps. It's a simple check, so it's unlikely to be done wrong and I'm not sure if we'll save any bytes. I'll go with whatever is smaller.

@robotdan
Copy link
Contributor

robotdan commented Jan 4, 2014

You beat me to the punch. :-) I have some unit tests that could be added to this if that is helpful. I'm using $._data to get the remove events, perhaps that isn't an ok thing to do in a unit test since that is an internal structure.

@scottgonzalez
Copy link
Member Author

Yeah, I don't want to look at $._data.

@scottgonzalez scottgonzalez deleted the tooltip-remove branch July 17, 2014 16:36
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.

3 participants