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
Reviewer Review Type Date Requested Status
Xavier (Open ERP) (community) Needs Fixing
Review via email: mp+211302@code.qastaging.launchpad.net

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/informations inside an openerp button, so that we can display any kind of useful information. Example: the number of invoices, the total sum invoiced for a client, or some kind of graph.

To post a comment you must log in.
Revision history for this message
Xavier (Open ERP) (xmo-deactivatedaccount) wrote :

# 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

review: Needs Fixing
3960. By Géry Debongnie

[IMP] various tweaks to stat_button, to fix issues mentioned in code review. This is mostly cosmetic cleanups, only notable change is that stat_button is now a proper html button instead of a label (addon web)

3961. By Géry Debongnie

[FIX] correctly handle the case where a 'stat button' has a string attribute (it needs to be displayed in a div instead of a span) (addon web)

3962. By Géry Debongnie

[IMP] improves the look of the stat buttons in form view: the statinfo widget displays the information on two lines, the padding/margin/width have been adjusted. (addon web)

3963. By Géry Debongnie

[MERGE] merge from trunk into local branch

3964. By Géry Debongnie

[IMP] removes the shadow in stat buttons, to give them the 'flat' look (addon web)

3965. By Géry Debongnie

[MERGE] merge from trunk

3966. By Géry Debongnie

[FIX] fixes an alignment issue in the template definitions (addon web)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.