-
Notifications
You must be signed in to change notification settings - Fork 94
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
i-bem: поправлен getMods() #1379
Conversation
Если посмотреть на окружающий метод |
Дело в том, что сейчас для без-DOM-ных элементов |
у |
Да, потому что у блоков без DOM-представления и нет элементов. |
Как мне кажется, что лучше провести более глубокий рефакторинг здесь:
|
Кажется, что если переопределить |
Ну так надо сделать так, чтобы не терялся ) |
1d37413
to
7b6e663
Compare
@dfilatov Поправил. |
* @param {Object} mods Modifiers values | ||
* @returns {BEM} | ||
*/ | ||
setCache: function(modNames, mods) { |
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.
Почему он публичный-то? @Protected мы начинаем также с _
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.
И зачем вообще отдельный метод, вроде можно заинлайнить это по месту.
1cadb68
to
0f70840
Compare
@dfilatov Спасибо! Поправил :) |
modCache = this._modCache, | ||
modNames = [].slice.call(arguments, hasElem ? 1 : 0); | ||
|
||
return !modNames.length ? |
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.
Странно что ругается хук, но у нас по cs "?" прибивается, должно быть return !modNames.length?
, поправь, пожалуйста, везде.
3f51947
to
734e79d
Compare
Поправил. |
@dfilatov нужен твой ок, сможешь посмотреть? |
Тесты не проходят же:
|
3b58f29
to
d0d553c
Compare
@narqo Поправил ) |
@@ -114,7 +114,18 @@ describe('i-bem', function() { | |||
delete BEM.blocks['block']; | |||
}); | |||
|
|||
describe('getMods', function() { | |||
it('should return specified mods', function() { | |||
block.getMods('mod1', 'mod2', 'mod3').should.be.deep.equal({ mod1 : 'val1', mod2 : true, mod3 : false }); |
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.
should.be.deep.equal
-> should.be.eql
По крайней мере, мы используем везде вторую версию.
d0d553c
to
81f3b0d
Compare
Поправил. |
У меня больше нет вопросов. |
@narqo есть ещё какие-то замечания? |
@dfilatov это в v4 нужно? |
@narqo там же нет |
По мотивам bem/bem-bl#591.
/cc @dfilatov @narqo