karlssonlord / KL_LessFriction

Checkout framework for Magento by Karlsson & Lord
GNU Lesser General Public License v2.1
0 stars 0 forks source link

JS Hooks should be on specific classes #10

Open JCB-K opened 11 years ago

JCB-K commented 11 years ago

When we write JS (in general, but in this case on LessFriction) we shouldn't be using the same classes for JS and CSS. I just fixed #9, where the problem was that somebody removed the .btn-remove class, because on Beans it's not a button but a link. What they didn't realise is that that broke the following JS:

var removeProduct = document.on(
    'click',
    '#shopping-cart-table .btn-remove',
    function(event, element) {
        checkout.queueRequest(element.href, {}, that._config);
        element.up('tr').remove();
        Event.stop(event);
    }.bind(this)
);

I think we're better off having specific classes for these hooks, such as .js-remove, or even use a LessFriction namespace: .js-lf-remove. Then when we're customizing templates for specific projects, we'll immediately understand the meaning of the class.

In order to do this we'll need to change all the listeners and classes in LessFriction, but also in the custom templates of each project that uses it.

indiebytes commented 11 years ago

Good point, I vote for js-remove or js-lessfriction-remove.

ptz0n commented 11 years ago

I think that .lf-remove is explicit enough.

JCB-K commented 11 years ago

@ptz0n It's explicit, but it doesn't work well with multiple front-end modules. Let's say we add KL_Foobar, we could use js-foobar-action for that.

indiebytes commented 11 years ago

@ptz0n @JCB-K I like the js prefix to make it clear it's not meant to be used for styling primarily. And I think the same name used in the modules config.xml should be used for consistency, in this case lessfriction.

ptz0n commented 11 years ago

Maybe a distinct camel cased handle, ex. .myHandle, instead of the all-lowercase-and–dashes class names for styling purposes?

Sidenote: This brings my mind to the directive thingy in Angular.