openerp-dev-web team mailing list archive

["Xavier \(Open ERP\)" <xmo@xxxxxxxxxxx>]

Review: Needs Fixing
* Why do you replace a JSON load by an eval (unrestricted too, not just a literal eval)?

* Why do you munge around with the filter value (removing '%') when the problem here is that this value should *not* have %'s in it because the server takes care of adding the '%' markers to ilike values?

* Could you explain what the problem was and why adding a conditional fixes it?

* from an efficiency standpoint, you added a conditional test within a loop. The loop is performed on the parent's children, and you test on the children's parent, so you could just perform the test on jQuery(this) outside the loop, which would avoid traversing the children and going up to the parent again. Either filter as now (via an if) or use .filter

* do not test visibility by checking for a CSS property, there is more to visibility than that and jQuery takes care of it via the `:visible` and `:hidden` pseudo-classes. They're also much clearer in terms of intent.
-- 
https://code.launchpad.net/~openerp-dev/openobject-client-web/trunk-improved-searchview-cpa/+merge/50113
Your team OpenERP R&D Team is subscribed to branch lp:~openerp-dev/openobject-client-web/trunk-improved-searchview-cpa.