BREAKING: simplify construct and add create method #70

qfox · 2017-01-26T18:39:19Z

blond

надо обновить документацию

blond · 2017-01-26T19:12:52Z

index.js

-     * @param {string} [obj.mod.name] — the modifier name of entity.
-     * @param {string} [obj.mod.val]  — the modifier value of entity.
+     * @param {string} obj.mod.name — the modifier name of entity.
+     * @param {string} obj.mod.val  — the modifier value of entity.


Почему стали обязательными полями?

Если mod передан, то name обязателен. val всё еще нет. Косяк

blond · 2017-01-26T19:22:47Z

test/create.test.js

+    t.is(entity.mod.name, 'mod1');
+});
+
+test('should use `mod.val` field instead of `modVal`', t => {


Можно ещё тестов:

mod: { name, val } + modVal + val

и mod: { name, val } + val

doneсыпал

blond · 2017-01-26T19:28:13Z

index.js

+     * @example
+     * const BemEntityName = require('@bem/entity-name');
+     *
+     * BemEntityName.create('my-button_theme_red');


рядом надо писать ахтунг-комментарий, что это id, а не naming

Supported only strings generated with id property. Написал в описание параметра

qfox · 2017-01-26T19:56:44Z

Досыпал документации

blond

мелкие замечания по документации

blond · 2017-01-26T20:18:33Z

README.md

-**Example:**
-
-```js
-// Boolean modifier of a block


Почему удалил этот пример?

я его перенес вниз же

blond · 2017-01-26T20:19:07Z

README.md

@@ -255,6 +222,57 @@ name.valueOf();

 Returns object for `JSON.stringify()` purposes.

+### #isBemEntityName(entityName)


Статичные методы сгруппировал?

blond · 2017-01-26T20:20:00Z

README.md

+
+Helper for laziness.
+
+**Important:** Only generated by id property strings are allowed.


Чот ощущуние, что можно передать только строку.

blond · 2017-01-26T20:20:58Z

README.md

+BemEntityName.create('my-button_theme_red');
+BemEntityName.create({ block: 'my-button', mod: 'theme', val: 'red' });
+BemEntityName.create({ block: 'my-button', modName: 'theme', modVal: 'red' });
+// BemEntityName { block: 'my-button', mod: { name: 'theme', val: 'red' } }


давай со стрелкой:

// ➜ BemEntityName { block: 'my-button', mod: { name: 'theme', val: 'red' } }

birhoff · 2017-02-02T22:39:18Z

README.md

@@ -75,6 +75,8 @@ new BemEntityName({
 });
 ```

+To describe the simple modifier, field `mod.val` must be specified as `true`.
+
 To describe the simple modifier field `mod.val` must be specified as `true`.


Do you think that people do not understand at first time? =)

birhoff · 2017-02-02T22:41:51Z

README.md

+const entityName = new BemEntityName({ block: 'input' });
+
+BemEntityName.isBemEntityName(entityName); // true
+BemEntityName.isBemEntityName({}); // false


Arrows // → true

birhoff · 2017-02-02T22:44:53Z

README.md

+`mod.name`   | `string` | The modifier name of entity.
+`mod.val`    | `*`      | The modifier value of entity.
+`modName`    | `string` | The modifier name of entity.
+`modVal`     | `*`      | The modifier value of entity.


modVal can be only String|True|undefined

birhoff · 2017-02-02T22:52:55Z

index.js

+     * @param {string} obj.block  — the block name of entity.
+     * @param {string} [obj.elem] — the element name of entity.
+     * @param {object|string} [obj.mod]  — the modifier of entity.
+     * @param {string} [obj.mod.name] — the modifier name of entity.


birhoff · 2017-02-02T22:54:07Z

index.js

+     * BemEntityName.create('my-button_theme_red');
+     * BemEntityName.create({ block: 'my-button', mod: 'theme', val: 'red' });
+     * BemEntityName.create({ block: 'my-button', modName: 'theme', modVal: 'red' });
+     * // BemEntityName { block: 'my-button', mod: { name: 'theme', val: 'red' } }


Here mate

// ➜

birhoff · 2017-02-02T22:56:15Z

index.js

+        if (mod || obj.modName) {
+            const isString = typeof mod === 'string';
+            const modName = (isString ? mod : mod && mod.name) || obj.modName;
+            const modObj = !isString && mod || obj;


Why is initial obj? There is no description about it. I think empty Object is better here.

qfox added the in progress label Jan 26, 2017

blond suggested changes Jan 26, 2017

View reviewed changes

qfox force-pushed the qfox.rework-majoritarity branch 7 times, most recently from f36fe69 to f52528a Compare January 26, 2017 21:14

birhoff reviewed Feb 2, 2017

View reviewed changes

qfox added 2 commits February 3, 2017 02:05

BREAKING: simplify construct and add create method

a6678d9

docs: update readme

7d116b2

qfox force-pushed the qfox.rework-majoritarity branch from f52528a to 7d116b2 Compare February 2, 2017 23:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BREAKING: simplify construct and add create method #70

BREAKING: simplify construct and add create method #70

qfox commented Jan 26, 2017

blond left a comment

blond Jan 26, 2017

qfox Jan 26, 2017

blond Jan 26, 2017

qfox Jan 26, 2017

blond Jan 26, 2017

qfox Jan 26, 2017

qfox commented Jan 26, 2017

blond left a comment

blond Jan 26, 2017

qfox Jan 26, 2017

blond Jan 26, 2017

blond Jan 26, 2017

blond Jan 26, 2017

birhoff Feb 2, 2017 •

edited

Loading

birhoff Feb 2, 2017

birhoff Feb 2, 2017

birhoff Feb 2, 2017

birhoff Feb 2, 2017

birhoff Feb 2, 2017

		@@ -255,6 +222,57 @@ name.valueOf();

		Returns object for `JSON.stringify()` purposes.

		### #isBemEntityName(entityName)


		Helper for laziness.

		Important: Only generated by id property strings are allowed.

BREAKING: simplify construct and add create method #70

Are you sure you want to change the base?

BREAKING: simplify construct and add create method #70

Conversation

qfox commented Jan 26, 2017

blond left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qfox commented Jan 26, 2017

blond left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

birhoff Feb 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

birhoff Feb 2, 2017 •

edited

Loading