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

fires closed.bs.alert *after* DOM detach() as per #12379 #13406

Merged
merged 2 commits into from
May 6, 2014

Conversation

EnsignR
Copy link
Contributor

@EnsignR EnsignR commented Apr 23, 2014

As per #12379

Fires closed.bs.alert after element is removed from DOM.
Previously it fired while the element was still attached.

fires closed.bs.alert *after* element is removed from DOM.
Previously it fired while the element was still attached.
@cvrebert cvrebert added the js label Apr 23, 2014
@cvrebert cvrebert added this to the v3.2.0 milestone Apr 23, 2014
@shirou
Copy link

shirou commented Apr 24, 2014

👍

@@ -42,7 +42,7 @@
$parent.removeClass('in')

function removeElement() {
$parent.trigger('closed.bs.alert').remove()
$parent.detach().trigger('closed.bs.alert')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a .remove() at the end, per #12379 (comment)

Choose a reason for hiding this comment

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

NAVER - http://www.naver.com/

lee36656@naver.com 님께 보내신 메일 <Re: [bootstrap] fires closed.bs.alert after DOM detach() as per #12379 (#13406)> 이 다음과 같은 이유로 전송 실패했습니다.


받는 사람이 회원님의 메일을 수신차단 하였습니다.


Added call to remove() after event fires to clean up data as per discussion at twbs#12379
@cvrebert
Copy link
Collaborator

cvrebert commented May 4, 2014

/to @fat for final review.

@fat
Copy link
Member

fat commented May 6, 2014

lgtm

@fat
Copy link
Member

fat commented May 6, 2014

thanks!

cvrebert added a commit that referenced this pull request May 6, 2014
fires closed.bs.alert *after* DOM detach() as per #12379
@cvrebert cvrebert merged commit a032c39 into twbs:master May 6, 2014
@mdo mdo mentioned this pull request May 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants