← Back to team overview

openerp-dev-web team mailing list archive

Re: lp:~openerp-dev/openobject-client-web/trunk-cal-events-readonly into lp:openobject-client-web

 

Review: Approve
> Ok. I did it but only for functions I added myself. I left the other one as is.

Yep, sensible, didn't expect you to fix everything.

Note that (for next time around maybe) jQuery's `hasClass` works on element sets, so

    jQuery(element).hasClass('is-not-droppable') || jQuery(element.parentNode).hasClass('is-not-droppable')

could also have been written

    jQuery([element, element.parentNode]).hasClass('is-not-droppable')

> Did it.

OK

> I started by adding an attribute to the div but I ran into some problems, which were probably related to something else. Since it's not always easy to debug javascript, I moved that atrribute to a new class. Perhaps that in the mean time, my problems were solved but I left that class. Now that code is running with the class, I could probably move it to an attribute again.

Class is OK, I didn't mean a DOM attribute, just that the code

    ${evt.droppable and 'is-droppable' or 'is-not-droppable'}

is repeated several time, and thus that maybe instead of having `evt.droppable` we could have `evt.droppable_class` with the right value and thus we could just write `${evt.droppable_class}`. Doesn't really matter.
-- 
https://code.launchpad.net/~openerp-dev/openobject-client-web/trunk-cal-events-readonly/+merge/39348
Your team OpenERP SA's Web Client R&D is requested to review the proposed merge of lp:~openerp-dev/openobject-client-web/trunk-cal-events-readonly into lp:openobject-client-web.



References