Merge lp://qastaging/~openerp-dev/openerp-web/trunk-proper-stat-buttons-ged into lp://qastaging/openerp-web
Proposed by
Géry Debongnie
Status: | Merged |
---|---|
Merged at revision: | 3966 |
Proposed branch: | lp://qastaging/~openerp-dev/openerp-web/trunk-proper-stat-buttons-ged |
Merge into: | lp://qastaging/openerp-web |
Diff against target: |
316 lines (+168/-15) 4 files modified
addons/web/static/src/css/base.css (+42/-6) addons/web/static/src/css/base.sass (+35/-4) addons/web/static/src/js/view_form.js (+79/-2) addons/web/static/src/xml/base.xml (+12/-3) |
To merge this branch: | bzr merge lp://qastaging/~openerp-dev/openerp-web/trunk-proper-stat-buttons-ged |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Xavier (Open ERP) (community) | Needs Fixing | ||
Review via email:
|
Description of the change
Stat buttons
Goal is to have useful and nice looking buttons in form views.
This branch allows the nesting of widgets/
To post a comment you must log in.
# Issues
* wouldn't removing `button` from the tags registry (and manually add the button tag to tags_to_init in process_button) and go through the generic `process_?` path simplify the code?
* `class` used to be a keyword, I'm not sure it works in supported MSIE versions, replacing `node.attrs.class` by `node.attrs[ 'class' ]` may avoid some risks
* `this.$().html` is equivalent to `this.$ el.find( ).html` , I'm surprised it works. Either way, `$()` whould be replaced by `$el`
# Possible improvements
* when defining this.is_ stat_button, a simple `and` would probably do the job, no need for a ternary expression
* using a regex test would be shorter than splitting then using an array lookup e.g. /\boe_stat_ button\ b/.test( node.attrs[ 'class' ])
* in the template, maybe split the conditional to keep the common code common? E.g. a conditional (possibly ternary) for the classes, and an other one for the inner content node? That way it might be easier to see what is and is not common between the two cases?
* in d3, both `.style` and `.attr` can take an object (of key: value) to avoid repeated calls to the method