Skip to content
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

Add documentation about notation #10

Merged
merged 2 commits into from
Mar 9, 2017
Merged

Add documentation about notation #10

merged 2 commits into from
Mar 9, 2017

Conversation

Yeti-or
Copy link
Member

@Yeti-or Yeti-or commented Mar 2, 2017

resolves: #4

@Yeti-or Yeti-or requested a review from veged March 2, 2017 19:10
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling cfe53ee on yeti-or.docs into 51116e0 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling ac82dd9 on yeti-or.docs into 51116e0 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 9c833a5 on yeti-or.docs into 51116e0 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling d45829b on yeti-or.docs into 51116e0 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling d45829b on yeti-or.docs into 51116e0 on master.

README.md Outdated
Parse your `import 'b:button2'`, yo!
Extract [bem-entities](https://en.bem.info/methodology/key-concepts/#bem-entity) from bem-import strings.

Install
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Installation

README.md Outdated
// { block : 'button', mod : { name: 'theme', val : 'action' } } ]
```

ToC:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Может быть лучше унести это в начало?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

И как отдельный раздел

README.md Outdated

parse('b:button e:text') // [ { block : 'button', elem : 'text' } ]

parse('b:button m:theme=normal|action') // [ { block : 'button' },
Copy link

@qfox qfox Mar 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я бы скинул на следующую строку и добавил после //

README.md Outdated
This section describes all possible syntax of bem-import strings.
Examples are provided in es6 syntax. Note that [parse](#api) function only works with strings.

Order of fields is important:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but why? Need to explain that

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of implementation ¯_(ツ)_/¯

README.md Outdated

```js
import 'b:button';
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a list of paths. Something like:

// → .../button/button.*

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

оно не раскрывается в пути, не думаю что нужно здесь это писать

README.md Outdated
```

#### block with several modifiers
```js
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

То есть пробел после заголовка, то нет

README.md Outdated

#### block with several modifiers
```js
import 'b:button e:text m:theme=active m:size=m';
Copy link

@qfox qfox Mar 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это раскрывается в:

// → .../button/button.*
// → .../button/__text/button__text.*
// → .../button/__text/_theme/button__text_theme.*
// → .../button/__text/_theme/button__text_theme_active.*
// → .../button/__text/_size/button__text_size.*
// → .../button/__text/_size/button__text_size_m.*

?

Если нет, то

  • почему важен порядок?
  • как подключить элемент другого блока с модификаторами?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Порядок важен из-за реализации
b:button e:text вернет тебе только элемент

не понял насчет элемента другого блока

Copy link

@qfox qfox Mar 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Например, в select2__item мы зовем 'b:menu e:item' (или около того).
Я забыл, что b:button e:text это не про блок. Вопрос снимается, невалидный.

Про реализацию всё еще не понятно как это может зависеть. Можно в первых проход найти все b:, потом e:, потом остальное.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

но мы делаем не так =) можешь посмотреть в код, там есть зависимость от того что пришло первое и если напишешь
t:css m:val b:button всё взорвётся, думаешь есть смысл это поддерживать?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

У нас же декларативное все, а здесь нет, это странно. Лучше явно проверять, что блок/элемент должен быть и только один. Там и ошибка пади непонятная сейчас

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Пофиксим вот здесь: #12
ты предлагаешь сейчас не писать про важность последовательности?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Да, потому что оно ничем не обусловлено (кроме странной реализации)

README.md Outdated

Technology is abstraction for extension on file system.

To extract bem-entities with special technology specify field `t:`.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specify field... to extract...

README.md Outdated

### technology

Technology is abstraction for extension on file system.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is an abstraction

Здесь лучше дать ссылку на key-concepts, там всё уже расписано.

README.md Outdated
License
-------

Code and documentation copyright 2014 YANDEX LLC. Code released under the [Mozilla Public License 2.0](LICENSE.txt).
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2014?
наверное, 2016–2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Скопируй из bem-react-core

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 9d35b88 on yeti-or.docs into 51116e0 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 410343c on yeti-or.docs into 51116e0 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 9dd5649 on yeti-or.docs into 51116e0 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling e3a231c on yeti-or.docs into 51116e0 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 36d0054 on yeti-or.docs into 51116e0 on master.

@Yeti-or Yeti-or merged commit a4b338e into master Mar 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add readme
4 participants