Item14537: The plugin makes tables "shaky"

pencil
Priority: Normal
Current State: Closed
Released In: 2.1.6
Target Release: patch
Applies To: Extension
Component: EditRowPlugin
Branches: Item14537 Release02x01 master Item14288
Reported By: VadimBelman
Waiting For:
Last Change By: GeorgeClark
With JS editing enabled the way the drag button on the left of draggable row increases its height sometimes. With some investigation I managed to track it down to erpJS_container <div> which has display: block property and appended into the end of enclosing <td> element list. For the moment I came up with the following patches for erp.uncompressed.js and erp.uncompressed.css files fixing the problem:

--- erp.uncompressed.js 2017-03-03 17:59:41.917544000 +0200
+++ erp.js      2017-11-23 03:54:29.594619000 +0200
@@ -315,9 +315,9 @@
                 var handle = $("<a href='#' class='ui-icon-arrow-2-n-s erp_drag_button ui-icon' title='Click and drag to move row'>move</a>");
                 // Note we need the extra <div class="erpJS_container" for positioning.
                 // It is *not* sufficient to set the class on the td
-                var div = $("<div class='erpJS_container'></div>");
+                var div = $("<div class='erpJS_container' style='display: inline-block; width: 1px;'></div>");
                 div.append(handle);
-                $(this).append(div);
+                $(this).prepend(div);
                 handle.draggable({
                     // constrain to the containing table
                     containment: container,

--- erp.uncompressed.css        2017-03-03 17:59:41.912718000 +0200
+++ erp.css     2017-11-23 04:26:00.062680000 +0200
@@ -23,19 +23,24 @@
     padding: 0;
 }

+.erpJS_cell {
+    display: inline-block;
+}
+
 .erp_drag_button {
     position: absolute;
     display: none;
     left: -1.7em;
-    top: -1.25em;
+    height: 1px;
 }

 a.erp_drag_button:hover {
     background-color: #FEE;
     position: absolute;
+    #display: inline-block;
     display: block;
     left: -1.7em;
-    top: -1.25em;
+    height: 1px;
 }

 .erp-button {

Briefly, what happens here is that the container and the cell <div> are changing their display propetries to inline-block. The container <div> is now inserted into the beginning of <td> element list in order to avoid 'guessing' about what left alignment to specify for the button's icon. The <div> is also made 1px narrow to minimize the horizontal shift of the cell content. Though event 0 width doesn't avoid the shift completely. Vertical alignment (top) is removed completely because the container is now guaranteed to be the topmost-left cell element.

The patch introduces another minor problem though. In Chrome 62 and Firefox 57 the drag button flickers when mouse pointer placed over it. Perhaps it's side effect of jQuery of some kind but I don't have time to fix this.

Otherwise dragging works as expected and other functionality of JS editing doesn't seem to be affected.

-- VadimBelman - 23 Nov 2017

Corrected patches to fix issues with tables being still shaky when they're packed with information and it is not possible to widen the first column cell. Also fixed incorrect positioning of the drag button when first cells in a row are not <td> but <th>.

--- erp.uncompressed.js   2017-11-23 21:57:30.000000000 -0500
+++ erp.js   2017-11-23 22:24:25.000000000 -0500
@@ -310,14 +310,14 @@
     var makeRowDraggable = function(tr) {
         // Add a "drag handle" element to the first cell of the row
         var container = tr.closest("thead,tbody,tfoot,table");
-        tr.find("td").first().each(
+        tr.find("td, th").first().each(
             function () {
                 var handle = $("<a href='#' class='ui-icon-arrow-2-n-s erp_drag_button ui-icon' title='Click and drag to move row'>move</a>");
                 // Note we need the extra <div class="erpJS_container" for positioning.
                 // It is *not* sufficient to set the class on the td
-                var div = $("<div class='erpJS_container'></div>");
+                var div = $("<div class='erpJS_container' style='display: inline-block; width: 0em;'></div>");
                 div.append(handle);
-                $(this).append(div);
+                $(this).prepend(div);
                 handle.draggable({
                     // constrain to the containing table
                     containment: container,
@@ -669,9 +669,9 @@
                 }
                 if (!current_row || tr[0] != current_row[0]) {
                     if (current_row) {
-                        current_row.find('.erpJS_container').fadeOut();
+                        current_row.find('.erpJS_container').fadeOut("fast");
                     }
-                    tr.find('.erp_drag_button,.erpJS_container').fadeIn();
+                    tr.find('.erp_drag_button,.erpJS_container').fadeIn("fast");
                     current_row = tr;
                 }
             });

--- erp.uncompressed.css   2017-11-23 21:57:30.000000000 -0500
+++ erp.css   2017-11-23 22:26:01.000000000 -0500
@@ -21,21 +21,29 @@
     position: absolute !important; /* Item13766 */
     margin: 0;
     padding: 0;
+    z-index: 1;
+    height: 50%;
+    background: rgba(255, 255, 255, 0);
+}
+
+.erpJS_cell {
+    display: inline-block;
 }

 .erp_drag_button {
     position: absolute;
     display: none;
     left: -1.7em;
-    top: -1.25em;
+    height: 1px;
 }

 a.erp_drag_button:hover {
     background-color: #FEE;
     position: absolute;
+    #display: inline-block;
     display: block;
     left: -1.7em;
-    top: -1.25em;
+    height: 1px;
 }

 .erp-button {
@@ -60,7 +68,7 @@
     width: 8px;
     background: url(icons.png) no-repeat transparent;
     background-position: -24px -35px;
-    z-index: 1;
+    z-index: 10;
     cursor: pointer;
 }

-- VadimBelman - 25 Nov 2017

Looks good. Recommend you apply the patch.

-- Main.CrawfordCurrie - 31 Jan 2018 - 10:20

Don't you have flickering when mouse pointer is over the drag button? I'd like to get rid of this effect in before applying.

-- VadimBelman - 31 Jan 2018

The fix is in Item14537 branch. Alongside with shakyness a couple of other related problems have been fixed too.

-- VadimBelman - 31 Jan 2018
 
Topic revision: r16 - 02 Mar 2018, GeorgeClark
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy