bem / bem-components

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

Radio-check and button mode #1685

Open awinogradov opened 8 years ago

awinogradov commented 8 years ago

Есть вот такой код: togglable : mods.mode === 'radio-check'? 'check' : 'radio'. Он находится вот здесь https://github.com/bem/bem-components/blob/v2/common.blocks/radio/_type/radio_type_button.bemhtml#L8 и встречается в checkbox и select. Вопроса у меня аж 2.

  1. Почему это не togglable : mods.mode?
  2. Почему это не mode : mods.mode?
sipayRT commented 8 years ago

Почему это не togglable : mods.mode?

Ну как-то блок radio с { mode : 'check' } - это херня, которая всех только больше запутает. А так тут можно понять, что это все же radio, который можно анчекнуть

Почему это не mode : mods.mode?

Потому что togglable тут уместнее, т.к. это название лучше описывает поведение

p.s.: это лично мое мнение

awinogradov commented 8 years ago

Ну как-то блок radio с { mode : 'check' } - это херня, которая всех только больше запутает. А так тут можно понять, что это все же radio, который можно анчекнуть

Это так люди сказали или вы сами придумали?)

Потому что togglable тут уместнее, т.к. это название лучше описывает поведение

Почему тогда это не togglable : mods.togglable?

awinogradov commented 8 years ago

Не вижу разницы между:

{ block: 'radio', mods: { mode : 'radio-check' } }

и

{ block: 'radio', mods: { mode : 'check' } }

Кажется, что причина не в понимании. Тем более что radio-check насильно выставляется в check.

sipayRT commented 8 years ago

Это так люди сказали или вы сами придумали?)

я ж специально в конце написал, что это имхо :)

Не вижу разницы

А я вижу :) Если бы был второй вариант, то ты бы спросил "Почему тогда не { block: 'check' } вместо { block: 'radio', mods: { mode : 'check' } }?".

awinogradov commented 8 years ago

Я бы спросил будь у меня такие мыслишки) Ответа пока еще нет(

narqo commented 8 years ago

Модификатор mode актуален только для select. В radio-group и checkbox-group он остался как рудимент, который специально не стали описывать в документации. На практике оказалось, что юзкейсы очень спорны, но блок был изначальных дизайнерских гайдах.

Модификатор mode намерено не описан в документации для radio и checkbox — он «технический» и нужен был, только для того, чтобы «прокинуть» правильное значение в кнопку.

Почему это не togglable : mods.mode

Потому что не бывает кнопки button_togglable_radio-check.

Почему это не mode : mods.mode

Это сематически-странно для кнопки, в которую прокидывается этот модификатор. togglable — свойство кнопки, она может быть либо западающей либо нет. Что такое «режим» (mode) для кнопки — непонятно.

awinogradov commented 8 years ago

Это не совсем верно. Не смотря на то, что думается togglable верным для кнопки, модификатор на кнопку ставиться checked при работе с mode внешних блоков. То есть верным было бы ставить button_toggled, если уж такая абстракция и понятно, что кнопка западающая или нет. Сейчас же кнопка выглядит так .button.button_togglable_check.button_checked.

Потому что не бывает кнопки button_togglable_radio-check.

Это больше похоже на недоработку, нежели на аргумент.

Что такое «режим» (mode) для кнопки — непонятно.

Очень субъьективно. Неужели правда не понятно как будет вести себя .button_mode_check?

Модификатор mode намерено не описан в документации

Шта?) Вы правда думаете, что пользователи не полезут внутрь шаблонов?

narqo commented 8 years ago

[..] То есть верным было бы ставить button_toggled, если уж такая абстракция и понятно, что кнопка западающая или нет.

Есть еще _togglable_radio. Все что можно выразить булевыми модификаторами — мы, конечно, выражаем булевыми модификаторами.

Потому что не бывает кнопки button_togglable_radio-check.

Это больше похоже на недоработку, нежели на аргумент.

Можешь описать, какое поведение должно быть у такой кнопки (при условии, что оно должно как-то согласовываться с поведением select_mode_radio-check)?

Модификатор mode намерено не описан в документации

Шта?) Вы правда думаете, что пользователи не полезут внутрь шаблонов?

То что кому-то может стать интересно как оно «устроено внутри» и он полезет смотреть приватные методы в JS-реализации, не делает эти методы публичными. Это правило масштабируется на все API блока, в том числе на модификаторы — что в этом странного?

awinogradov commented 8 years ago

Странность на лицо. В виде текущего issue.

Есть еще _togglable_radio. Все можно выразить булевыми модификаторами — мы, конечно, выражаем булевыми модификаторами.

Это же не противоречит .button_toggled никак. Тем не менее, когда кнопка западает, она получает такой набор .button.button_togglable_radio.button_checked. Кажется, что это сильно непонятнее, чем например .button.button_togglable_radio.button_toggled или .button.button_mode_radio.button_toggled. И одинаково не понятно с .button.button_mode_radio.button_checked.

Можешь описать, какое поведение должно быть у такой кнопки (при условии, что оно должно как-то согласовываться с поведением select_mode_radio-check)?

Тут скорее проблема не в функционале кнопки, а в имени select_mode_radio-check. Из него напрямую не следует, что можно выбрать ничего. Поэтому и транслировать это на кнопку сложно. Недоработка не в кнопке. К транслированию radio-check относительно безконтрольности. Как по мне надо было падать с ошибкой.

Вы поймите, я тут без наездов. Просто хочется чуть более очевидную логику работы компонентов.

voischev commented 8 years ago

mode для кнопки кажется более понятным

narqo commented 8 years ago

Про toggled vs checked я понял, но не уверен, что с этим есть серьезные проблемы (либо, просто «внутри», за годы все привыкли к такой терминологии, поэтому никогда не жаловались). По крайней мере, на сколько я знаю, ассоциации с

<input type="radio" checked/>
<input type="checkbox" checked/>

плотно сидят у всех в головах. Да и во всех остальных блоках (checkbox, radio, select, menu) используются те же термины.

Недоработка не в кнопке. К транслированию radio-check относительно безконтрольности. Как по мне надо было падать с ошибкой

Тут я ничего не понял, но для radio-group_mode_radio-check нужна западающая кнопка (та что button_togglable_check). Больше случаев использования mods.mode === 'radio-check'? 'check' : 'radio' я не нашел. В любом случае, сейчас это рудимет, который нельзя удалить в 2.x.

Про неинтуитивность названия модификатора кнопки togglable я услышал (хотя не согласен, что _mode в этом смысле понятнее — тогда уж _togglable-mode). Предлагаю изменить заголовок таска на что-то осмысленное и связанное с переименовыванием button_togglable, и отложить до времен когда будет планироваться 3.x. В 2.x модификаторы переименовывать мы не будем.

awinogradov commented 8 years ago

Я не против поменять заголовок, но как назвать ума не приложу)

aristov commented 8 years ago

button_togglable_radio - ошибка проектирования. Эта логика должна быть реализована в radio и radio-group.

radio_type_button следует переименовать в radio_view_button и использовать button только как стилевой миксин.

button_togglable_check => button_togglable - тогглбатон.

checkbox_type_button => checkbox_view_button - не должен зависеть от button_togglable и наследовать его логику, только подмешивать на себя стили button и button_pressed.

awinogradov commented 8 years ago

@aristov :fire:

voischev commented 8 years ago

@aristov :+1: :fire: :fire_engine: