Dropdown switcher link and button strange behaviour

[qfox]

There is a weak condition in dropdown_switcher_link and dropdown_switcher_button templates: https://github.com/bem/bem-components/blob/v2/common.blocks/dropdown/_switcher/dropdown_switcher_link.bh.js#L5 https://github.com/bem/bem-components/blob/v2/common.blocks/dropdown/_switcher/dropdown_switcher_button.bh.js#L5

At my POV it should also check for size of array. If it contains only one block — it should parse it as well as not an array.

So code should be like:

        if(Array.isArray(content)) {
            if (content.lentgh > 1) return content;
            if (content.length === 1) content = content[0];
        }

Example:

({
    block : 'dropdown',
    mods : { switcher : 'button' },
    switcher : [ { block: 'button', mods: { disabled: true } } ],
    popup : 'bemjson'
});

Expected result:

<button class="button button__control ...

Actual:

<button class="button button_disabled button__control ...

[qfox]

Also look at https://github.com/zxqfox/bem-components/blob/feature/php-bh-templates%40v2/common.blocks/dropdown/_switcher/dropdown_switcher_link.bh.php#L7

One more case: If I pass there an empty array — it will fail too.

[narqo]

Why should anyone want to wrap the lite version of button's BEMJSON into an Array. IMHO this kind of things should not be encouraged by any sort of APIs, until it doesn't really hurt anyone.

We try to avoid to solve "because we can" problems, until a real world problem occurred.

[qfox]

Well, this kind of troubles is very annoying and undesirable.

What if it will be a helpers in bemhtml/bh.js to simplify content to array of object, or null (if array is empty), or object (if array's length is 1)?

Also it's hard to repeat this functionality in another platforms with strict typing where content should be an array or array-like object like in bh.php.

I'm open for any ideas, and this PR just to show exactly what I meant.

[narqo]

What if it will be a helpers in bemhtml/bh.js to simplify content to array of object, or null (if array is empty), or object (if array's length is 1)?

I don't think that I understood what you mean by the phrase above.

One more case: If I pass there an empty array — it will fail too.

An empty array means that your usage of the block is incorrect, sorry. This kind of thinks must be solved in the API documentation level (BTW, we have PRs with docs fixed, so stay tuned ;)

The only real problem that we tried to solve was to have an ability to wrap __switcher in the unknown something. In this case in looks fine to insist to have a full declaration of the switcher, and not to try to predict what did user want.

Don't you think that switcher : [ { block: 'button', mods: { disabled: true } } ] is this kind of case?

P.S. To be honest, from my own experience, all this polymorphism stuff doesn't have any application in the real world projects. But at the same time it brings additional responsibility to the maintainers, as they should provide an API which is consistent, stable and fully documented ;)

[qfox]

I don't think that I understood what you mean by the phrase above.

I mean something like bh.toContent (and this.toContent for bemhtml) which would convert content node to array or object if it hold an array of 1 element ([{block: 'example'}]).

The only real problem that we tried to solve was to have an ability to wrap __switcher in the unknown something. In this case in looks fine to insist to have a full declaration of the switcher, and not to try to predict what did user want.

Don't you think that switcher : [ { block: 'button', mods: { disabled: true } } ] is this kind of case?

Nope because this part of code:

if(res.block === 'button') {
 // ...
}

I think these parts are very similar so atm I don't see consistency in templates. And I think they need some refactoring.

Moreover these bh.js templates can't be automatically converted into bh.php templates. Just one more reason to do it.

[narqo]

Nope because this part of code [..] I think these parts are very similar so atm I don't see consistency in templates.

For me, the real inconsistency would come if only one itemed array would be treated as a special case.

The logical question from the users would be: "what the hell with other array cases":

[ 
  { block : 'button', mods : { disabled : true } },   // why not to check first item in array?
  { block : 'icon', ... }
]

[, { block : 'button', mods : { disabled : true } }]   // note "hole" in first position

[ 
  [{ block : 'button', mods : { disabled : true } }]   // note nested array here (it's a valid BEMJSON)
]

The questions like above are always asked from our internal users. And it's really hard to write valid documentation for such API decisions (as well as proof it to users ;) From this point even current __switcher's API is hard to understand :(

So, the point is: "until there are no real cases which make block almost unusable, no API changes are welcome". We decided to formalise it in #1208 in the nearest future.

Moreover these bh.js templates can't be automatically converted into bh.php templates. Just one more reason to do it.

Sorry, but I don't think that ability to port current codebase to other (unsupported) languages should be a reason to make any kind of API changes.

[veged]

agree with @narqo that portability should not affect code such much — if needed it can be done only in BH.php templates