Review: Needs Fixing
Looks pretty good, but:
* Paginator is not correctly synced when switching from form after having
opened the view once:
- Open workflows
- Switch to diagram view
- Switch to form view
- Go to page 5
- Switch to diagram view
The diagram view correctly displays diagram #5, but the paginator say "1/7".
However it jumps straight to 6/7 when clicking [Next] pagination button
* Do you need the `t-att-id="element_id + '_header'"` in the template? I could
not find a use for it
* Incorrect naming conventions for methods of Workflow class, method names
should be lowercase and underscore_separated, not PascalCase
* Could you try using the Class.include method to mix objects in your classes?
Or just use delegation? Instead of $.extend calls I mean.
- also generally speaking we prefer using underscore's "general-purpose"
functions to jQuery's (e.g. extend, map, each, etc…) when possible, this also
done in EditRecord method ($.each)
* there are some bad or debatable pieces of code:
- I'm not sure the `select_node` method is still used, and it calls removed
method `add_edit_node`
- in EditRecord, `id` is defined as an implicit global, pretty sure that is
an error
* ShowDialog result, keys are hardcoded to english so will not be translatable.
Also why define `result` to `{}` and then use `$.extend` to set its content in
exclusive situations, instead of just creating empty/undefined result and then
assigning?
* Also `null`, `false`, `""` and `undefined` can not be instances of a
user-defined type so writing `foo and foo instanceof MyObject` is redundant.
--
https://code.launchpad.net/~openerp-dev/openerp-web/trunk-diagram-draw2d/+merge/91420
Your team OpenERP R&D Team is subscribed to branch
lp:~openerp-dev/openerp-web/trunk-diagram-draw2d.
_______________________________________________
Mailing list: https://launchpad.net/~openerp-dev-gtk
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openerp-dev-gtk
More help : https://help.launchpad.net/ListHelp
