Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

fix(tooltip): clean up stackedMap on scope destroy #4610

Merged
merged 1 commit into from
Oct 13, 2015
Merged

fix(tooltip): clean up stackedMap on scope destroy #4610

merged 1 commit into from
Oct 13, 2015

Conversation

RobJacobs
Copy link
Contributor

The tooltip was not being removed from the stackedMap
resulting in a memeory leak. Tooltip will now be
removed from the stackedMap in scope destroy
function.

Fixes #4604

The tooltip was not being removed from the stackedMap
resulting in a memeory leak.  Tooltip will now be
removed from the stackedMap in scope destroy
function.

Closes #4610

Fixes #4604
@wesleycho
Copy link
Contributor

LGTM

@Foxandxss
Copy link
Contributor

Please, add tests.

@wesleycho
Copy link
Contributor

Not sure we can add a test here, the stacked map is not exposed publicly.

@Foxandxss
Copy link
Contributor

You inject the stacked map in the test, you add a few tooltips, you destroy one, you check that the stack has one tooltip less.

@wesleycho
Copy link
Contributor

But the stack that needs to be checked is the internal one - that is the one that gets removed

@RobJacobs RobJacobs merged commit ebb5e18 into angular-ui:master Oct 13, 2015
@RobJacobs RobJacobs deleted the fix-tooltip-stackedmap branch October 13, 2015 18:30
@Foxandxss
Copy link
Contributor

@RobJacobs tell us your secrets to close a PR as merged like you did without creating extra unwanted history messages.

@RobJacobs
Copy link
Contributor Author

@Foxandxss When working in a PR branch, after I make the initial commit I always use 'git commit --amend' to keep my commit history clean. Then when I merge to master, rebase the PR branch from master, then in the bootstrap repository:

git checkout master
git pull <fork-url> <branch>
git push origin

aroop pushed a commit to aroop/bootstrap that referenced this pull request Oct 16, 2015
The tooltip was not being removed from the stackedMap
resulting in a memeory leak.  Tooltip will now be
removed from the stackedMap in scope destroy
function.

Closes angular-ui#4610

Fixes angular-ui#4604
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants