From 7e221a929e122834f1359f38333c577f59656b5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Quentin=20Harsco=C3=ABt?= Date: Mon, 29 Mar 2021 01:12:19 +0200 Subject: [PATCH 1/2] Fix buggy sorting in dataexplorer_table.php Table sorting had errors : - Current Live version uses parseInt to sort values so don't sort numbers that have the same integer part - At least on today's (03/28) values, had some weird bug : when sorting by ascending weekly growth, mayotte's -60,5% and St-Pierre-et-Miquelon's -100,0% were somewhere in the middle and seemed to cause some reset in the sorting as further down the values seemed in the correct order. Same thing when changing the order around Moselle's +0,00. Didn't want to debug the custom sorting algorithm so I used javascript's Array.sort() function, works like a charm and even improved performance. --- src/DataExplorer/dataexplorer_table.php | 93 +++++++++---------------- 1 file changed, 34 insertions(+), 59 deletions(-) diff --git a/src/DataExplorer/dataexplorer_table.php b/src/DataExplorer/dataexplorer_table.php index 93d16ed7..f6978304 100644 --- a/src/DataExplorer/dataexplorer_table.php +++ b/src/DataExplorer/dataexplorer_table.php @@ -414,7 +414,8 @@ function sortTable(idxToSort, order) { lastsort = idxToSort lastorder = order - if(lastorder=="asc"){ + // Down arrow is supposed to be for descending order I guess, grom highest to lowest + if(lastorder=="desc"){ document.getElementById('col'+'0').innerHTML = "▽" document.getElementById('col'+'1').innerHTML = "▽" document.getElementById('col'+'2').innerHTML = "▽" @@ -429,65 +430,39 @@ function sortTable(idxToSort, order) { } - var table, rows, switching, i, x, y, shouldSwitch; - table = document.getElementById("myTable"); - //console.log(table.innerHTML) - switching = true; - /*Make a loop that will continue until - no switching has been done:*/ - var j = 0 - while (switching) { - //start by saying: no switching is done: - switching = false; - rows = table.rows; - //console.log(rows) - /*Loop through all table rows (except the - first, which contains table headers):*/ - for (i = 1; i < (rows.length - 1); i++) { - //start by saying there should be no switching: - shouldSwitch = false; - /*Get the two elements you want to compare, - one from current row and one from the next:*/ - x = rows[i].getElementsByTagName("TD")[idxToSort]; - y = rows[i + 1].getElementsByTagName("TD")[idxToSort]; - //check if the two rows should switch place: - assess=false - - if(idxToSort>0){ - if(order=="desc"){ - //console.log("___") - //console.log(idxToSort) - //console.log(x.getElementsByTagName('span')[1].innerHTML) - - assess = (parseInt(x.getElementsByTagName('span')[1].innerHTML.toLowerCase()) > parseInt(y.getElementsByTagName('span')[1].innerHTML.toLowerCase())) - } else if(order=="asc") { - assess = (parseInt(x.getElementsByTagName('span')[1].innerHTML.toLowerCase()) < parseInt(y.getElementsByTagName('span')[1].innerHTML.toLowerCase())) - } - } else if(idxToSort==0){ - if(order=="desc"){ - //assess = (parseInt(x.innerHTML.toLowerCase()) < parseInt(y.innerHTML.toLowerCase())) - assess = (('' + x.getElementsByTagName('span')[0].innerHTML).localeCompare(y.getElementsByTagName('span')[0].innerHTML)) - assess = (assess == 1) - } else if(order=="asc") { - //assess = (parseInt(x.innerHTML.toLowerCase()) > parseInt(y.innerHTML.toLowerCase())) - assess = (('' + y.getElementsByTagName('span')[0].innerHTML).localeCompare(x.getElementsByTagName('span')[0].innerHTML)) - assess = (assess == 1) - } - } - if (assess==true) { - //if so, mark as a switch and break the loop: - shouldSwitch = true; - break; - } - } + var table, rows, x, y; + table = document.getElementById("myTable"); - if (shouldSwitch) { - /*If a switch has been marked, make the switch - and mark that a switch has been done:*/ - rows[i].parentNode.insertBefore(rows[i + 1], rows[i]); - switching = true; - } - } + + + function compareRows(row1,row2) { + x = row1.getElementsByTagName("TD")[idxToSort]; + y = row2.getElementsByTagName("TD")[idxToSort]; + + if(idxToSort > 0){ + // Put back decimal point and use ParseFloat + x = parseFloat(x.getElementsByTagName('span')[1].innerHTML.toLowerCase().replace(",",".")); + y = parseFloat(y.getElementsByTagName('span')[1].innerHTML.toLowerCase().replace(",",".")); + if(x == NaN) return 1; + if(y == NaN) return -1; + + if(order == "desc") + return y - x; + else if(order == "asc") + return x - y; + } + else if(idxToSort == 0) { + if(order == "desc") + return (('' + y.getElementsByTagName('span')[0].innerHTML).localeCompare(x.getElementsByTagName('span')[0].innerHTML)); + else if(order == "asc") + return (('' + x.getElementsByTagName('span')[0].innerHTML).localeCompare(y.getElementsByTagName('span')[0].innerHTML)); + + } + } + + // Used built-it javascript sort function instead of risking using a handwritten algorithm that might have mistakes + rows = Array.from(table.rows).slice(1); + rows.sort(compareRows).forEach(function(r) {r.parentNode.appendChild(r);}); } From 2aaf15fa54853c8de85ff0c7dd87f84b87af3a28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Quentin=20Harsco=C3=ABt?= Date: Mon, 29 Mar 2021 01:48:14 +0200 Subject: [PATCH 2/2] Handle "+-0.0" that could happen --- src/DataExplorer/dataexplorer_table.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/DataExplorer/dataexplorer_table.php b/src/DataExplorer/dataexplorer_table.php index f6978304..51ca9e2d 100644 --- a/src/DataExplorer/dataexplorer_table.php +++ b/src/DataExplorer/dataexplorer_table.php @@ -440,9 +440,9 @@ function compareRows(row1,row2) { y = row2.getElementsByTagName("TD")[idxToSort]; if(idxToSort > 0){ - // Put back decimal point and use ParseFloat - x = parseFloat(x.getElementsByTagName('span')[1].innerHTML.toLowerCase().replace(",",".")); - y = parseFloat(y.getElementsByTagName('span')[1].innerHTML.toLowerCase().replace(",",".")); + // Put back decimal point and use ParseFloat, and take care of "+-0,0" value that caused a NaN + x = parseFloat(x.getElementsByTagName('span')[1].innerHTML.toLowerCase().replace(",",".").replace("+-", "")); + y = parseFloat(y.getElementsByTagName('span')[1].innerHTML.toLowerCase().replace(",",".").replace("+-", "")); if(x == NaN) return 1; if(y == NaN) return -1;