bem / bem-xjst

bem-xjst (eXtensible JavaScript Templates): declarative template engine for the browser and server
https://bem.github.io/bem-xjst
Other
116 stars 48 forks source link

Ability to pass attribute style as object #232

Closed tenorok closed 8 years ago

tenorok commented 8 years ago

Ability to pass object for attribute style can be very useful, because we will be able to extend this object and it is cool sugar – we free from necessary concatenate strings.

I think that we should realize next moments:

  1. Property name can be specified with minus (text-align) or as camel case (textAlign)
  2. Property value can be specified as number – in this case we mean px (except zero)
  3. At the end of style list should be a semicolon for avoid sticking in the future

For example:

attrs()({
    style: {
        width: 100, // 100px
        height: 0, // 0
        'text-align': 'center',
        verticalAlign: 'top' // vertical-align
    }
})

And result:

style="width:100px;height:0;text-align:center;vertical-align:top;"

Example of realization: https://github.com/tenorok/bemer/blob/master/modules/Tag.js#L295-L305 And of tests: https://github.com/tenorok/bemer/blob/master/test/TagTest.js#L136-L153

L0stSoul commented 8 years ago

Я так думаю что ядро фичи заключается в следующем: Очень хочется не конкатенировать эту строку руками. Если например через инлайновые стили нужно задачть ширину и высоту, у нас получается какой то такой код:

if (data.width) {
    block.attrs.style += 'width:' + data.width + 'px;';
}

хочется иметь возможность писать что то похожее:

block.attrs.style = {
     width: data.width 
}

Это поидее полностью эквивалентные куски кода :)

Тоесть поведение должно быть как у условного поля mix или чего то такого, его можно в bemjson передать, в шаблонах написать и поведение будет всегда нормик :)

Все остальное это полезный обвес, но не суперкритикал.

L0stSoul commented 8 years ago

В общем +1;

xoxulin commented 8 years ago

zIndex: 1001 => z-index: 1001px, also flex-grow|shrink|order etc.

xoxulin commented 8 years ago

More exceptions => more code => more time :(

xoxulin commented 8 years ago

Если дотачивать до исключений (z-index etc.), то добавится много кода - придется знать дефолтные юниты для css свойств - оверхед. Может быть стоит подумать на счет расширяемости за счет плагинов / полифилов / сахара, а не хардкода?!

tenorok commented 8 years ago

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

miripiruni commented 8 years ago

Никакого сахара (вида автодобавления px к числу в значениях к определенным свойствам) я не обещаю.

miripiruni commented 8 years ago

Логично сделать просто механизм сериализации { block: 'b', attrs: { style: { …} } }, чтобы избавиться от избыточного кода при генерации атрибутов.

sladex commented 8 years ago

+1 Автодобавление px по-моему лишнее, оверхэд и все равно не покроет все случаи.

Про расширяемость можно в эту сторону подумать:

attrs()({
    style: {
        width: {
          value: 100,
          validate: (value) => { return value >= 0; },
          format: (value) => { return value + 'px'; }
        },
        color: {
          value: '#f00',
          validate: (value) => { return /#([a-f0-9]{6}|[a-f0-9]{3})\b/gi.test(value) } // цвет строго в hex формате
        },
        'font-family': {
          value: 'Arial',
          format: (value) => { return value + ', sans-serif' }
        },
        'text-align': {
          value: 'center',
          validate: (value) => { return ['left', 'center', 'right'].indexOf(value) !== -1; } // разрешаем выставлять не все валидные значения (еще есть justify, initial и т.д.), а только определенные 
        },
        'background-image': {
          value: '//yastatic.net/morda-logo/i/bender/logo.png',
          validate: String, // а что, тоже функция...
          format: (value) => { return 'url(' + value + ')' }
        }
    }
})
kaero commented 8 years ago

Почему для этого нельзя сделать просто функцию вне bem-xsjt, которая сериализует такой объект в строку? Почему это должно быть в шаблонизаторе, генерирующем разметку? Более того, готов поставить сотку на то, что кто-то такое уже написал.

Guria commented 8 years ago

угу https://www.npmjs.com/package/to-style

var toStyleString = require('to-style').string

templates.apply({
  block: 'b',
  attrs: {
    style: toStyleString({
      border: {
        width: 1,
        color: 'red'
      },
      padding: 4,
      margin: {
        top: 5
      }
    })
  }
})

ну или:

block('*')
  .match(function () { return this.ctx.style })
  .attrs()(function () {
    return {
        style: toStyleString(this.ctx.style)
    }
  })
miripiruni commented 8 years ago

Почему для этого нельзя сделать просто функцию вне bem-xsjt, которая сериализует такой объект в строку? Почему это должно быть в шаблонизаторе, генерирующем разметку?

Можно. Но. Ко мне пришли независимо друг от друга для человека @L0stSoul и @tenorok и просят сделать это в самом bem-xjst.

По моим оценкам запилить фичу в самом базовом варианте (см PR) заняло 15 минут. Поддерживать фичу не составит особого труда (по-моему, она достаточно четкая если говорить только про базовую сериализацию block.attrs.style {…}). Поэтому не понимаю зачем заставлять разработчиков дописывать какую-то функцию, если рано или поздно всем это может понадобиться, а проекту это стоит три копейки. Я не прав?

kaero commented 8 years ago

Такими темпами у тебя тут появятся локализации, css-препроцессор и grep — это все тоже очень нужные и полезные штуки, зуб даю.

xoxulin commented 8 years ago

Более того, готов поставить сотку на то, что кто-то такое уже написал.

https://github.com/facebook/react/blob/25481083b0138fe052fcd00c591dc5e10e0a64d6/src/renderers/dom/shared/dangerousStyleValue.js - да)

miripiruni commented 8 years ago

@kaero убедил.

@L0stSoul @tenorok покажите, пожалуйста, примеры шаблонов, в которых хочется этой фичи.

L0stSoul commented 8 years ago

Почему для этого нельзя сделать просто функцию вне bem-xsjt, которая сериализует такой объект в строку?

Можно конечно, так же можно сделать функцию которая сериализует в строку поля mix, elem, и block, и не создавать их отдельными полями шаблонизатора.

Где правильная граница ? идея в том что из таких мелких неудобств складывается в целом сложность использования. Тут нам нужно похачить, а это из коробки.

@miripiruni примеры кода: https://nda.ya.ru/3RYRWb https://nda.ya.ru/3RYRWd https://nda.ya.ru/3RYRZJ

miripiruni commented 8 years ago

@L0stSoul а теперь смотри.

blocks['nevermind'] = function(context) {
    ...
    return {
        block: 'serp-item',
        elem: 'title',
        attrs: {
            style: blocks['styles'](context)
        },
        content: content
    };
};

blocks['styles'] = function(context) {
    if (context.isPremium || !context.expFlags.direct_right_title_size) return;

    var fontSize = parseInt(context.expFlags.direct_right_title_size);

    return 'font-size:' + fontSize + 'px;line-height:' + fontSize * 1.125 + 'px';
};

Допустим я волью PR в текущем виде. Код второго шаблона можно будет переписать в виде:

return {
    'font-size': fontSize + 'px;',
    'line-height': fontSize * 1.125 + 'px';
};

Как решится проблема c if для проверки содержимого полей? Никак :)

Поэтому, похоже что нужно активнее использовать шаблоны и match(). Всё как я рассказывал на поледнем докладе на Пятнице. Там были хорошие примеры.

Use pattern matchng, Luke.

// priv
blocks['nevermind'] = function(context) {
    ...
    return {
        block: 'example',
        size: (context.isPremium || !context.expFlags.direct_right_title_size) || parseInt(context.expFlags.direct_right_title_size),
        content: content
    };
};

+

// BEMHTML Template:
blocks('nevermind')
    .match()(function() { return this.ctx.size; })
    .attrs()(function() {
        return { style: 'font-size:' + fontSize + 'px;line-height:' + fontSize * 1.125 + 'px'; }
    })
);
miripiruni commented 8 years ago

I suggest to use third-party package to stringily styles hash. Look at to-style or similar. You can use bemcontext extend API for connect such packages.

Feel free to reopen this issue or open the new one.

tenorok commented 8 years ago

@kaero всё таки описанная в этой задаче возможность относится непосредственно к шаблонизации, в отличии от локализации, css-препроцессоров и грепа :-)

У нас вот сейчас нет простого способа подключить отдельный пакет в код, сам знаешь. Это надо ковырять сборку. Ну а про плохие стороны использования require в рантайме ты сам говорил.

tenorok commented 8 years ago

@kaero а так то можно и БЭМ-шаблонизатор не писать. Есть же отдельный пакет для CSS-классов: https://github.com/albburtsev/bem-cn — и не один такой 😄

qfox commented 8 years ago

Ну а про плохие стороны использования require в рантайме ты сам говорил.

Я ошибаюсь, или сейчас ядро bem-xjst собирается именно реквайрами через browserify? Т.е. мы пользуемся плохими сторонами?