From 79c197814b5eb399975f799bf538ae7bf61f4db8 Mon Sep 17 00:00:00 2001 From: Gery Debongnie Date: Thu, 8 May 2014 13:00:27 +0200 Subject: [PATCH 1/5] [IMP] improve the way indentation is done in pivot table (addon web_graph) Previously, it was done in a ugly way: added several spans with the class .graph_indent. Now, it simply sets the margin of the content. bzr revid: ged@openerp.com-20140508110027-bdjdzlpfptjuf4fa --- addons/web_graph/static/src/css/graph.css | 4 ---- addons/web_graph/static/src/js/graph_widget.js | 4 +--- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/addons/web_graph/static/src/css/graph.css b/addons/web_graph/static/src/css/graph.css index 25ee5db9594..6bc628f2eb7 100644 --- a/addons/web_graph/static/src/css/graph.css +++ b/addons/web_graph/static/src/css/graph.css @@ -96,10 +96,6 @@ span.field-selection { padding: 0; } -span.web_graph_indent { - padding-left: 30px; -} - .web_graph_click:hover { cursor: pointer; } diff --git a/addons/web_graph/static/src/js/graph_widget.js b/addons/web_graph/static/src/js/graph_widget.js index 9946e149d96..0efb0a57ce2 100644 --- a/addons/web_graph/static/src/js/graph_widget.js +++ b/addons/web_graph/static/src/js/graph_widget.js @@ -543,15 +543,13 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ var content = $('').addClass('web_graph_click') .attr('href','#') .text(' ' + (header.title || _t('Undefined'))) + .css('margin-left', header.indent*30 + 'px') .attr('data-id', header.id); if (_.has(header, 'expanded')) { content.addClass(header.expanded ? 'fa fa-minus-square' : 'fa fa-plus-square'); } else { content.css('font-weight', 'bold'); } - if (_.has(header, 'indent')) { - for (var i = 0; i < header.indent; i++) { cell.prepend($('', {class:'web_graph_indent'})); } - } return cell.append(content); }, From b9c217e8500e27babaf3ddc6745e37724eff9932 Mon Sep 17 00:00:00 2001 From: Gery Debongnie Date: Thu, 8 May 2014 13:13:11 +0200 Subject: [PATCH 2/5] [FIX] add $ to jquery variables (addon web_graph) the rendering code of the pivot table didn't use $ in jquery variables, this commit fixes that issue and slightly simplify their names. bzr revid: ged@openerp.com-20140508111311-kargzvzlttutwt47 --- .../web_graph/static/src/js/graph_widget.js | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/addons/web_graph/static/src/js/graph_widget.js b/addons/web_graph/static/src/js/graph_widget.js index 0efb0a57ce2..64f043014d7 100644 --- a/addons/web_graph/static/src/js/graph_widget.js +++ b/addons/web_graph/static/src/js/graph_widget.js @@ -540,51 +540,51 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ .addClass('graph_border') .attr('rowspan', header.height) .attr('colspan', header.width); - var content = $('').addClass('web_graph_click') + var $content = $('').addClass('web_graph_click') .attr('href','#') .text(' ' + (header.title || _t('Undefined'))) .css('margin-left', header.indent*30 + 'px') .attr('data-id', header.id); if (_.has(header, 'expanded')) { - content.addClass(header.expanded ? 'fa fa-minus-square' : 'fa fa-plus-square'); + $content.addClass(header.expanded ? 'fa fa-minus-square' : 'fa fa-plus-square'); } else { - content.css('font-weight', 'bold'); + $content.css('font-weight', 'bold'); } - return cell.append(content); + return cell.append($content); }, draw_headers: function (headers, doc_fragment) { var make_cell = this.make_header_cell, - empty_cell = $('').attr('rowspan', headers.length), - thead = $(''); + $empty_cell = $('').attr('rowspan', headers.length), + $thead = $(''); _.each(headers, function (row) { - var html_row = $(''); + var $row = $(''); _.each(row, function (header) { - html_row.append(make_cell(header)); + $row.append(make_cell(header)); }); - thead.append(html_row); + $thead.append($row); }); - thead.children(':first').prepend(empty_cell); - doc_fragment.append(thead); - this.thead = thead; + $thead.children(':first').prepend($empty_cell); + doc_fragment.append($thead); + this.$thead = $thead; }, draw_measure_row: function (measure_row, doc_fragment) { if (this.pivot.measures.length === 1) { return; } - var html_row = $('').append(''); + var $row = $('').append(''); _.each(measure_row, function (cell) { - var measure_cell = $('').addClass('measure_row').text(cell.text); - if (cell.is_bold) {measure_cell.css('font-weight', 'bold');} - html_row.append(measure_cell); + var $cell = $('').addClass('measure_row').text(cell.text); + if (cell.is_bold) {$cell.css('font-weight', 'bold');} + $row.append($cell); }); - this.thead.append(html_row); + this.$thead.append($row); }, draw_rows: function (rows, doc_fragment) { var make_cell = this.make_header_cell, cell, - html_row, i, j; + $row, i, j; var cells_list, hcell, @@ -592,7 +592,7 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ rows_length = rows.length; for (i = 0; i < rows_length; i++) { - html_row = $('').append(make_cell(rows[i])); + $row = $('').append(make_cell(rows[i])); cells_length = rows[i].cells.length; cells_list = []; @@ -608,8 +608,8 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ hcell += '>' + cell.value + ''; cells_list[j] = hcell; } - html_row.append(cells_list.join('')); - doc_fragment.append($('').append(html_row)); + $row.append(cells_list.join('')); + doc_fragment.append($('').append($row)); } }, From 1c87d5b6f4edc58f6990f65fb78e88772d72f538 Mon Sep 17 00:00:00 2001 From: Gery Debongnie Date: Thu, 8 May 2014 13:27:23 +0200 Subject: [PATCH 3/5] [FIX] render the pivot table correctly (addon web_graph) The code was bugged: it added a tag around each rows instead of adding the rows to a single tbody. bzr revid: ged@openerp.com-20140508112723-b6ffkmufwux1z0a7 --- addons/web_graph/static/src/js/graph_widget.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/addons/web_graph/static/src/js/graph_widget.js b/addons/web_graph/static/src/js/graph_widget.js index 64f043014d7..fb738d5d593 100644 --- a/addons/web_graph/static/src/js/graph_widget.js +++ b/addons/web_graph/static/src/js/graph_widget.js @@ -591,8 +591,13 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ rows_length, cells_length; rows_length = rows.length; + + var $tbody = $(''); + doc_fragment.append($tbody); for (i = 0; i < rows_length; i++) { - $row = $('').append(make_cell(rows[i])); + $row = $('') + .attr('data-indent', rows[i].indent) + .append(make_cell(rows[i])); cells_length = rows[i].cells.length; cells_list = []; @@ -609,7 +614,7 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ cells_list[j] = hcell; } $row.append(cells_list.join('')); - doc_fragment.append($('').append($row)); + $tbody.append($row); } }, From 8f55e4519fe26c677bc1ad1bc3b0ee72d03731a4 Mon Sep 17 00:00:00 2001 From: Gery Debongnie Date: Thu, 8 May 2014 14:40:02 +0200 Subject: [PATCH 4/5] [IMP] optimize the pivot table rendering when folding rows (addon web_graph) don't redraw the full table when folding rows. instead, it removes from the dom the corresponding rows. This should give a big speed boost when folding rows on big tables. bzr revid: ged@openerp.com-20140508124002-yaa383rq4c0qhrf8 --- .../web_graph/static/src/js/graph_widget.js | 28 +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/addons/web_graph/static/src/js/graph_widget.js b/addons/web_graph/static/src/js/graph_widget.js index fb738d5d593..1a7c1c1d45f 100644 --- a/addons/web_graph/static/src/js/graph_widget.js +++ b/addons/web_graph/static/src/js/graph_widget.js @@ -294,7 +294,11 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ self = this; if (header.expanded) { - this.fold(header); + if (header.root === this.pivot.rows) { + this.fold_row(header, event); + } else { + this.fold_col(header); + } return; } if (header.path.length < header.root.groupby.length) { @@ -353,9 +357,27 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ }, - fold: function (header) { - var update_groupby = this.pivot.fold(header); + fold_row: function (header, event) { + var rows_before = this.pivot.rows.headers.length, + update_groupby = this.pivot.fold(header), + rows_after = this.pivot.rows.headers.length, + rows_removed = rows_before - rows_after; + if (rows_after === 1) { + // probably faster to redraw the unique row instead of removing everything + this.display_data(); + } else { + var $row = $(event.target).parent().parent(); + $row.nextAll().slice(0,rows_removed).remove(); + } + if (update_groupby && this.graph_view) { + this.graph_view.register_groupby(this.pivot.rows.groupby, this.pivot.cols.groupby); + } + }, + + fold_col: function (header) { + var update_groupby = this.pivot.fold(header); + this.display_data(); if (update_groupby && this.graph_view) { this.graph_view.register_groupby(this.pivot.rows.groupby, this.pivot.cols.groupby); From e94c1f03d66ef165b14183d0fe3ff0eee75f5aa8 Mon Sep 17 00:00:00 2001 From: Gery Debongnie Date: Fri, 9 May 2014 11:09:35 +0200 Subject: [PATCH 5/5] [IMP] optimizes pivot table rendering when expanding rows (addon web_graph) This patch makes the graph widget only add the rows that were new instead of redrawing the full table when expanding bzr revid: ged@openerp.com-20140509090935-sje3u6i4x1luswwn --- .../web_graph/static/src/js/graph_widget.js | 87 +++++++++++-------- 1 file changed, 49 insertions(+), 38 deletions(-) diff --git a/addons/web_graph/static/src/js/graph_widget.js b/addons/web_graph/static/src/js/graph_widget.js index 1a7c1c1d45f..d0038452dfb 100644 --- a/addons/web_graph/static/src/js/graph_widget.js +++ b/addons/web_graph/static/src/js/graph_widget.js @@ -70,8 +70,9 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ self.graph_view.register_groupby(self.pivot.rows.groupby, self.pivot.cols.groupby); } }); - openerp.web.bus.on('click', self, function () { + openerp.web.bus.on('click', self, function (event) { if (self.dropdown) { + self.$row_clicked = $(event.target).closest('tr'); self.dropdown.remove(); self.dropdown = null; } @@ -302,6 +303,7 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ return; } if (header.path.length < header.root.groupby.length) { + this.$row_clicked = $(event.target).closest('tr'); this.expand(id); return; } @@ -349,10 +351,22 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ groupby = groupby || header.root.groupby[header.path.length]; this.pivot.expand(header_id, groupby).then(function () { + if (header.root === self.pivot.rows) { + // expanding rows can be done by only inserting in the dom + // console.log(event.target); + var rows = self.build_rows(header.children); + var doc_fragment = $(document.createDocumentFragment()); + rows.map(function (row) { + doc_fragment.append(self.draw_row(row)); + }); + self.$row_clicked.after(doc_fragment); + } else { + // expanding cols will redraw the full table + self.display_data(); + } if (update_groupby && self.graph_view) { self.graph_view.register_groupby(self.pivot.rows.groupby, self.pivot.cols.groupby); } - self.display_data(); }); }, @@ -398,7 +412,7 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ return { headers: this.build_headers(), measure_row: this.build_measure_row(), - rows: this.build_rows(raw), + rows: this.build_rows(this.pivot.rows.headers,raw), nbr_measures: this.pivot.measures.length, title: this.title, }; @@ -463,7 +477,7 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ return cell; }, - build_rows: function (raw) { + build_rows: function (headers, raw) { var self = this, pivot = this.pivot, m, i, j, k, cell, row; @@ -471,11 +485,11 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ var rows = []; var cells, pivot_cells, values; - var nbr_of_rows = pivot.rows.headers.length; + var nbr_of_rows = headers.length; var col_headers = pivot.get_cols_leaves(); for (i = 0; i < nbr_of_rows; i++) { - row = pivot.rows.headers[i]; + row = headers[i]; cells = []; pivot_cells = []; for (j = 0; j < pivot.cells.length; j++) { @@ -592,7 +606,7 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ this.$thead = $thead; }, - draw_measure_row: function (measure_row, doc_fragment) { + draw_measure_row: function (measure_row) { if (this.pivot.measures.length === 1) { return; } var $row = $('').append(''); _.each(measure_row, function (cell) { @@ -603,40 +617,37 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ this.$thead.append($row); }, - draw_rows: function (rows, doc_fragment) { - var make_cell = this.make_header_cell, - cell, - $row, i, j; - - var cells_list, - hcell, - rows_length, cells_length; + draw_row: function (row) { + var $row = $('') + .attr('data-indent', row.indent) + .append(this.make_header_cell(row)); - rows_length = rows.length; + var cells_length = row.cells.length; + var cells_list = []; + var cell, hcell; - var $tbody = $(''); - doc_fragment.append($tbody); - for (i = 0; i < rows_length; i++) { - $row = $('') - .attr('data-indent', rows[i].indent) - .append(make_cell(rows[i])); - cells_length = rows[i].cells.length; - cells_list = []; - - for (j = 0; j < cells_length; j++) { - cell = rows[i].cells[j]; - hcell = ''; - cells_list[j] = hcell; + for (var j = 0; j < cells_length; j++) { + cell = row.cells[j]; + hcell = ''; + cells_list[j] = hcell; + } + return $row.append(cells_list.join('')); + }, + + draw_rows: function (rows, doc_fragment) { + var rows_length = rows.length, + $tbody = $(''); + + doc_fragment.append($tbody); + for (var i = 0; i < rows_length; i++) { + $tbody.append(this.draw_row(rows[i])); } },