bem / bem-components

Set of components for sites development
https://bem.info/libraries/classic/bem-components/6.0.0/
Other
333 stars 87 forks source link

radio: bug with form serialize #329

Closed aliosv closed 10 years ago

aliosv commented 10 years ago

radio.change -> form.serialize() -> выводит пустое значение радиогруппы https://github.com/aliosv/project-stub/tree/aliosv.bugRadio (npm i, bem server, desktop.bundles/index/index.html) в примере с нативными инпутами все ок http://jsfiddle.net/54tnx/

P.s. метод getName в контролах перекрывает аналогичный из i-bem.js, баг?

narqo commented 10 years ago

radio.change -> form.serialize() -> выводит пустое значение радиогруппы

У тебя в примере this.domElem — это <span>. jQuery разве умеет самостоятельно искать дочерний HTMLInputElement перед сериализацией?

Сравни с this.elem('control').serialize() ;)

Мы как-то обсуждали, что хорошо бы иметь API для сериализации формы, а-ля Form#toJson(), но до реализации еще не добрались. Давай про это на встрече поговорим.

P.s. метод getName в контролах перекрывает аналогичный из i-bem.js, баг?

getName() в i-bem.js это метод класса, в контролах — это метод инстанса.

aliosv commented 10 years ago

https://github.com/aliosv/project-stub/blob/aliosv.bugRadio/desktop.bundles/index/index.bemjson.js#L21 tag: form

в i-bem.js это метод класса, в контролах — это метод инстанса.

спасибо

narqo commented 10 years ago

tag: form

Действительно, не заметил.

Проблема в том, что БЭМ-событие change триггерится до того, как реально выставляется checked на внутреннем input'е (\cc @dfilatov).

С другой стороны, $.serialize — это достаточно «грубое копание в потрохах» блоков. Никто не гарантирует, что radio-option__control всегда будет реализован через <input type="radio">.

В данном случае (пока bem-components не предоставляет средств сериализации), следовало бы использовать Radio#getVal().

modules.define(
  'test', 
  ['i-bem__dom', 'radio', 'checkbox'], 
  function(provide, BEMDOM, Radio, Checkbox) {

provide(BEMDOM.decl(this.name, {
  onSetMod: {
    'js' : {
      'inited' : function() {
        [Radio, Checkbox].forEach(function(blockClass) {
          blockClass.on(this.domElem, 'change', function(e) { 
            console.log(e.target.getVal());
          });
        }, this);
      }
    }
  }
}));

});
tadatuta commented 10 years ago

я контекстно завел https://github.com/bem/bem-components/issues/328 про передачу значения на change всегда для всех контролов. сейчас где-то есть, где-то нет.

narqo commented 10 years ago

@tadatuta как это поможет с сериализацией формы?

tadatuta commented 10 years ago

@narqo это поможет на текущий момент, пока сериализации нет, чтобы вместо e.target.getVal() консистентно получать значение через второй аргумент в коллбеке для всех контролов.

narqo commented 10 years ago

Проблема в том, что БЭМ-событие change триггерится до того, как реально выставляется checked на внутреннем input'е

@aliosv завели про это баг в bem/bem-core#454 Войдет в bem-core@2.1.0

narqo commented 10 years ago

bem/bem-core#464