popup: split own functionality into target modifiers (close #968) #1007
Conversation
| return this; | ||
| }, | ||
|
|
||
| /** | ||
| * Sets content | ||
| * @param {String|jQuery} content | ||
| * @returns {popup} this | ||
| */ | ||
| setContent : function(content) { |
There was a problem hiding this comment.
Do we actually need this method in common api? Maybe move it to _target?
There was a problem hiding this comment.
How are _target and setContent related to each other?
There was a problem hiding this comment.
setContent is valuable because it does 2 things at the same time: updates the DOM and redraws the popup. Since we need to redraw the popup only in _target_* modifications, this method valuable only in this modifications. in popup.js it only updates the DOM and loses value, because we can do that just through BEMDOM.update(popup.domElem, content).
There was a problem hiding this comment.
it's inconsistent to have two ways to update content of popup (which apply differently depend on just modifier)
There was a problem hiding this comment.
Hmm, would you implement the setContent() method, if you weren't need to .redraw() popup in the _target_*?
There was a problem hiding this comment.
Yes, I would. It's often case in applications I'm developing.
|
Tests are passed! Could we squash it and merge? |
narqo
force-pushed
the
issues/968@v2
branch
from
2d7a350
to
a319b61
Compare
|
I don't see any reason not to merge it ) |
popup: split own functionality into target modifiers (close #968)
No description provided.