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

ya-share2, move yashare, bem-contrib #74

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

adinvadim
Copy link

#73
#68

Не было пакета enb-magic-factory в devDeps
bem-incubator -> bem-contrib

По поводу тестов для ya-share2, как сгенерировать html?

@voischev
Copy link
Member

@adinvadim не хочешь просто сделать апгрейд блока yashare?
Не очень нравится циферки в названии блоков

@voischev
Copy link
Member

@teryaew если сделать полный апдейт яшар... у вас же ничего не сломается?

@adinvadim
Copy link
Author

@voischev
Под update подразумевается полный отказ от поддержки старого yandex share? или возможность изменять контекстно с помощью поля ctx.apiVersion?

@voischev
Copy link
Member

@adinvadim думаю правильно будет отказываться от поддержки старых версий и держать актуальное только новое АПИ. Так что да... просто вырезаем оставляем привычное имя

}
]
block : 'yashare',
quickServices : [
Copy link
Author

Choose a reason for hiding this comment

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

Оставить поддержку поля quickServices, которое в новом api просто services, или тоже вырезать?

Copy link
Member

Choose a reason for hiding this comment

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

@adinvadim думаю двигаться нужно на упрощение) так что вырезай

@adinvadim
Copy link
Author

Думаю нужно добавить еще вот это функционал https://tech.yandex.ru/share/doc/dg/add-docpage/#separately

@voischev
Copy link
Member

@adinvadim добавляй. В верном направлении думаешь 👍

@adinvadim
Copy link
Author

@voischev

JShint ругается на правило forin (https://jslinterrors.com/the-body-of-a-for-in-should-be-wrapped-in-an-if-statement), хотя я и так делаю проверку на hasOwnProperty, но делаю немножко по другому, с отрицанием. Если я делаю, так как он просит, то ругается на глубокую вложенность. И как поступить в таком случае, не использовать for in?

...
        var separately = {
            'data-description' : ctx.description,
            'data-image' : ctx.image,
            'data-title' : ctx.title,
            'data-url' : ctx.url,
        },
        _obj;


        for(var field in separately) {
            if(!separately.hasOwnProperty(field)) continue;
            _obj = separately[field];
            if(typeof _obj === 'string') continue;
            if(_obj instanceof Object) {
                separately[field] = _obj['default'];
                for(var social in _obj) {
                    if(!_obj.hasOwnProperty(social)) continue;
                    if(social === 'default') continue;
                    separately[field + ':' + social] = _obj[social];
                }
            }
        }
        attrs = this.extend(attrs, separately);
        return attrs;
...

По поводу решения в плане API этого функционала, то такие поля как
image, title, url, description могут принмать String или Object.

И выглядит это примерно так

...
                    title : {
                        default : 'BEM Social Components Library',
                        vkontakte : 'BEM Social Components Library; (from vk share)',
                        twitter : 'BEM Social Components Library; (from twitter share)'
                    },
                    description : 'Fork me on GitHub',
...

@voischev
Copy link
Member

voischev commented Apr 3, 2016

Я думаю ты можешь отключить это правило в линтере

cls()('ya-share2'),
attrs()(function() {
var ctx = this.ctx,
attrs = {
Copy link
Member

Choose a reason for hiding this comment

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

кмк, либо дописать еще один var, либо отбить все объявление вправо.

но вообще предлагаю что-нибудь типа:

var ctx = this.ctx;

return ['bare', 'counter', 'copy', 'description', 'image', 'lang', 'limit', 'size', 'title', 'url'].reduce(function(prev, attr) {
    prev['data-' + attr] = ctx[attr];
    return prev;
}, { 'data-services': (ctx.services || []).join(',') });

@tadatuta
Copy link
Member

tadatuta commented Apr 3, 2016

@adinvadim

JShint ругается на правило forin

А можно весь код целиком посмотреть? У меня есть устойчивое ощущение, что его можно заметно упростить с помощью функциональщины типа Object.keys().reduce(), но из приведенного сниппета не очень понятно, какая задача решается.

@adinvadim
Copy link
Author

@tadatuta

Согласен Object.keys().reduce() тут то что нужно

block('yashare')(
    js()(true),
    cls()('ya-share2'),
    attrs()(function() {
        var ctx = this.ctx,
            attrs = {
                'data-bare' : ctx.bare,
                'data-counter' : ctx.counter,
                'data-copy' : ctx.copy,
                'data-lang' : ctx.lang,
                'data-limit' : ctx.limit,
                'data-services' : (ctx.services || []).join(','),
                'data-size' : ctx.size,
            },
            separately = {
                'data-description' : ctx.description,
                'data-image' : ctx.image,
                'data-title' : ctx.title,
                'data-url' : ctx.url,
            },
            _obj;

        for(var field in separately) {
            if(!separately.hasOwnProperty(field)) continue;
            _obj = separately[field];
            if(typeof _obj === 'string') continue;
            if(_obj instanceof Object) {
                separately[field] = _obj['default'];
                for(var social in _obj) {
                    if(!_obj.hasOwnProperty(social)) continue;
                    if(social === 'default') continue;
                    separately[field + ':' + social] = _obj[social];
                }
            }
        }
        attrs = this.extend(attrs, separately);
        return attrs;
     })
);
 title : {
    default : 'BEM Social Components Library',
    vkontakte : 'BEM Social Components Library; (from vk share)',
    twitter : 'BEM Social Components Library; (from twitter share)'
}

Вернет
data-title="BEM Social Components Library" data-title:vkontakte="..." etc

@tadatuta
Copy link
Member

tadatuta commented Apr 4, 2016

@adinvadim
я не тестировал, но примерно так:

block('yashare')(
    js()(true),
    cls()('ya-share2'),
    attrs()(function() {
        var ctx = this.ctx;
        var attrs = [
            'bare', 'counter', 'copy', 'description', 'image',
            'lang', 'limit', 'size', 'title', 'url'
        ].reduce(function(prev, attr) {
            prev['data-' + attr] = ctx[attr];
            return prev;
        }, { 'data-services': (ctx.services || []).join(',') });

        ['description', 'image', 'title', 'url'].reduce(function(prev, attr) {
            var val = ctx[attr];

            if (typeof val === 'string') {
                prev[attr] = val;
            } else {
                Object.keys(val).forEach(function(key) {
                    prev[key === 'default' ? attr : (attr + ':' + key)] = val[key];
                });
            }
            return prev;
        }, attrs);

        return attrs;
     })
);

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.

3 participants