openerp-dev-web team mailing list archive
-
openerp-dev-web team
-
Mailing list archive
-
Message #00009
Re: [Merge] lp:~vda-tiny/openobject-client-web/drag-drop_groupby into lp:~openerp-dev/openobject-client-web/trunk-dev-web
> Technical:
>
> * I think this branch could have been in more than a single commit (for
> instance moving the import in listgrid.mako should have been its own commit).
> Doesn't matter for now, but remember for future if possible.
> * attributes spacing (PEP8) e.g. listgroup.mako, line 125, there should be
> spaces around `!=`. Likewise line 150, there should be a space after the `,`
> in the itertools.chain call, or line 37 in the `map` call.
> * listgrid.js, in dragRow, `var args` should be moved down a few lines and
> created entirely via literal syntax:
>
> var args = {
> '_terp_model': _terp_model.value,
> '_terp_ids': _terp_ids.value,
> '_terp_id': getNodeAttribute(drag, 'record'),
> '_terp_swap_id': getNodeAttribute(drop, 'record'),
> };
>
ok, i will do it.
> * listgrid.py lines 248+
> - return should be `{}` (simpler, faster than `dict`)
> - don't use **kw when you know all arguments passed to method, clearer to
> specify keyword arguments:
>
> def groupbyDrag(self, **kw):
> domain = eval(kw.get('domain'))[0]
> model = kw.get('model')
> children = eval(kw.get('children'))
> =>
> def groupbyDrag(self, model, domain, children):
> - what happens if kw['model'] is empty? Or domain, or children? No need to
> use .get if it's going to blow up anyway, just do direct getitem (kw['model'])
> to get error as clear as possible (e.g. KeyError, not SyntaxError or
> TypeError)
> - we're just evaluating literal Python data types (list, strings,
> integers, …) => please use ast.literal_eval
> (http://docs.python.org/library/ast.html#ast.literal_eval) not eval as eval is
> more risky (if client performs direct json call to method using eval, can
> create damage or even control server, not good, with literal_eval no chance to
> do that as it only accepts lists, dicts, strings, numbers, booleans and None,
> no risk of unrestricted object access or anything)
>
Nice.
> UI/UX:
> * so it's only to move tasks from one groupby to another?
> * page reloads and all groups redisplay closed (except the one that
> disappeared when dragging a group on another one), very bad experience
> * not very fast
> * it's very weird that moving a groupby onto another groupby merges them, is
> that really what's supposed to happen? Is there spec somewhere?
we are following gtk. so it will work like gtk.
--
https://code.launchpad.net/~vda-tiny/openobject-client-web/drag-drop_groupby/+merge/22799
Your team OpenERP SA's Web Client R&D is subscribed to branch lp:~openerp-dev/openobject-client-web/trunk-dev-web.