← Back to team overview

openerp-dev-web team mailing list archive

Re: lp:~openerp-dev/openobject-client-web/improved-custom-filters into lp:openobject-client-web

 

Second round of review, after that everything should be good I think

Remarks on current state:
* In actions, "And" is title-cased but "OR" is uppercased. "Or" should be titlecased as well.
* td should not have style, but should have class="item"
* Custom fields needs a pair of small fixes:
	- prefix id to avoid conflict with main search form (e.g. in Leads,  there is input#name in both search field and custom files)
	- Field label (string) should be in an actual <label> with @for set to prefixed ID of item above, that way we can click on name not just checkbox
* Small bug: in Leads, when I click on "state" to hide the colums, produces 500 error with stack trace ending with:
  File "tiny/web/current/addons/openerp/widgets/listgrid.py", line 257, in display
    return super(List, self).display(value, **params)
  File "tiny/web/current/openobject/widgets/_base.py", line 194, in display
    return tools.render_template(self.template_c, d)
  File "tiny/web/current/openobject/tools/_expose.py", line 191, in render_template
    return utils.NoEscape(template.render_unicode(**kw))
  File "tiny/web/current/lib/python2.6/site-packages/mako/template.py", line 198, in render_unicode
    as_unicode=True)
  File "tiny/web/current/lib/python2.6/site-packages/mako/runtime.py", line 403, in _render
    _render_context(template, callable_, context, *args, **_kwargs_for_callable(callable_, data))
  File "tiny/web/current/lib/python2.6/site-packages/mako/runtime.py", line 434, in _render_context
    _exec_template(inherit, lclcontext, args=args, kwargs=kwargs)
  File "tiny/web/current/lib/python2.6/site-packages/mako/runtime.py", line 457, in _exec_template
    callable_(context, *args, **kwargs)
  File "_addons_openerp_widgets_templates_listgrid_mako", line 255, in render_body
  File "_addons_openerp_widgets_templates_listgrid_mako", line 32, in make_row
  File "_addons_openerp_widgets_templates_listgrid_mako", line 626, in render_make_row
  File "tiny/web/current/addons/openerp/widgets/listgrid.py", line 612, in params_from
    state = ((state or False) and state.value) or 'draft'
AttributeError: 'unicode' object has no attribute 'value'

Revision-specific remarks:
sma@xxxxxxxxxxx-20100826045842-ppna4v95uictu23d:
* No need to specify colspan when 1, that's the default
* Total colspan should be same in all lines (otherwise not all lines are same size): filter rows total 5 colspans (3*1 + 2) but actions row only has 4 (2+2). Colspans should all be removed from filter rows, only leave those on actions row.
	
Also width on first item of filter rows (image_col) doesn't seem to do anything, should be removed.

sma@xxxxxxxxxxx-20100827061641-myrzxvwgd6or8kc9:
* cust_domains is defined in `if custom_domains` so I think if there are no custom_domains last line will crash with NameError because cust_domains won't exist. `cust_domains` declaration should be lifted out of the conditional (to where inner_domain declaration was setup)
* Parens around get_domain() calls are unnecessary no?
* Python boolean contexts should be able to call len() on sequences when it needs, so e.g. line 306 could be
	if val:
  no?

sma@xxxxxxxxxxx-20100830073926-qo6pb78xzm9s0mvp:
function search_filter, first create `var custom_columns` as empty string then next line overwrite it entirely. Why not merge two lines?

That's about it, thanks for the good work
-- 
https://code.launchpad.net/~openerp-dev/openobject-client-web/improved-custom-filters/+merge/32975
Your team OpenERP R&D Team is subscribed to branch lp:~openerp-dev/openobject-client-web/improved-custom-filters.



References