-
Notifications
You must be signed in to change notification settings - Fork 95
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
Fix unbind on block destruct #1581
base: v4
Are you sure you want to change the base?
Conversation
@@ -250,9 +250,11 @@ var undef, | |||
ctxStorage = eventStorage[ctxId] = {}; | |||
if(isBindToInstance) { | |||
ctx._events().on({ modName : 'js', modVal : '' }, function() { | |||
params.bindToArbitraryDomElem && ctxStorage[storageKey] && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@veged может bindToArbitraryDomElem
полностью из параметров удалить? Он использовался только в этом месте.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Этот параметр появился после этого фикса d095501
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
звучит логично. добавишь отрывание отдельным коммитом?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Если @veged не против, то добавлю. В переписке телеги были сомнения на этот счёт.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
я не понимаю, почему хочется его удалять? это же про ветку, когда мы биндимся к произвольной jQuery-цепочке
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@veged потому что оно не используется. Точнее использовалось непонятно для чего.
При каждой новой связке через _events()
создаётся новый мендежер для текущего контекста.
При этом подписка на отписку происходит только при первой связке и то если имеет этот ключ.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
«используется непонятно для чего» != «не используется» ;-)
я ж говорю, bindToArbitraryDomElem
используется при байнде к произвольной jQuery-цепочке
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
так тут и было это использование — я поэтому и не понимаю, как ты смог его удалить ;-) нет спек на байнд к произвольной jQuery-цепочке получается :-(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
d1c8214
to
a978ac7
Compare
fix #1580