-
Notifications
You must be signed in to change notification settings - Fork 96
i-bem: поправлен getMods() #1379
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
Conversation
|
Если посмотреть на окружающий метод |
|
Дело в том, что сейчас для без-DOM-ных элементов |
|
у |
|
Да, потому что у блоков без DOM-представления и нет элементов. |
|
Как мне кажется, что лучше провести более глубокий рефакторинг здесь:
|
|
Кажется, что если переопределить |
Ну так надо сделать так, чтобы не терялся ) |
|
@dfilatov Поправил. |
common.blocks/i-bem/i-bem.vanilla.js
Outdated
| * @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.
И зачем вообще отдельный метод, вроде можно заинлайнить это по месту.
|
@dfilatov Спасибо! Поправил :) |
common.blocks/i-bem/i-bem.vanilla.js
Outdated
| 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?, поправь, пожалуйста, везде.
|
Поправил. |
|
@dfilatov нужен твой ок, сможешь посмотреть? |
|
Тесты не проходят же:
|
|
@narqo Поправил ) |
common.blocks/i-bem/i-bem.spec.js
Outdated
|
|
||
| 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
По крайней мере, мы используем везде вторую версию.
|
Поправил. |
|
У меня больше нет вопросов. |
|
@narqo есть ещё какие-то замечания? |
|
@dfilatov это в v4 нужно? |
|
@narqo там же нет |
По мотивам bem/bem-bl#591.
/cc @dfilatov @narqo