neo icon indicating copy to clipboard operation
neo copied to clipboard

component.Carousel: itemCls

Open tobiu opened this issue 3 years ago • 0 comments

Hi Torsten,

I merged your PR. The itemClick logic seems to be copied from a list and feels a bit odd to me.

config:

        /**
         * Custom cls added to each item
         * This is only a single string
         *
         * @member {String|null} itemCls=null
         */
        itemCls: null,

domListeners:

            click: {
                fn      : me.onClick,
                delegate: '.neo-carousel-item',
                scope   : me
            }

i guess neo-carousel-item should be the itemCls

    onClick(data) {
        let me = this,
            item;

        if (data.path[0].id === me.id) {
            me.onContainerClick(data);
        } else {
            for (item of data.path) {
                if (item.cls.includes(me.itemCls)) {
                    me.onItemClick(item, data);
                    break;
                }
            }
        }
    }

since we delegate to a static itemCls => neo-carousel-item, the container click if case can never happen. the else case will only trigger in case a dev does set its custom iconCls on top of the default one.

tl-br: using neo-carousel-item as the default value for itemCls and polishing the onClick() logic feels needed.

thoughts?

tobiu avatar Aug 10 '22 07:08 tobiu