Merge pull request #634 from sbcgua/master

Stage page UX improvements, polishing

src/zabapgit_css_common.w3mi.data.css

@@ -47,7 +47,9 @@ input:focus, textarea:focus {
 .crossout         { text-decoration: line-through !important; }
 .right            { text-align:right; }
 .paddings         { padding: 0.5em 0.5em; }
-.pad-sides        { padding: 0 0.3em; }
+.pad-sides        { padding-left: 0.3em; padding-right: 0.3em; }
+.margin-v5        { margin-top: 0.5em; margin-bottom: 0.5em; }
+.indent5em        { padding-left: 0.5em; }
 .pad4px           { padding: 4px; }
 .w100             { width: 100%; }
 .w40              { width: 40%; }
@@ -360,6 +362,9 @@ table.repo_tab {
 }
 
 /* STAGE */
+div.stage-container { width: 850px; }
+input.stage-filter { width: 18em; }
+
 .stage_tab {
   border: 1px solid #DDD;
   background: #fff;
@@ -388,6 +393,8 @@ table.repo_tab {
   color: #444 !important;
   font-weight: bold;
 }
+
+.stage_tab td.cmd {  cursor: pointer; }
 .stage_tab td.cmd a { padding: 0px 4px; }
 .stage_tab th.cmd a { padding: 0px 4px; }
 .stage_tab td.method { color: #ccc; }

src/zabapgit_html_chunks.prog.abap

@@ -209,7 +209,9 @@ CLASS lcl_gui_chunk_lib IMPLEMENTATION.
   METHOD render_js_error_banner.
     CREATE OBJECT ro_html.
     ro_html->add( '<div id="js-error-banner" class="dummydiv error">' ).
-    ro_html->add( |{ lcl_html=>icon( 'alert/red' ) } JS init error, please log an issue| ).
+    ro_html->add( |{ lcl_html=>icon( 'alert/red' ) }| &&
+                  ' If this does not disappear soon,' &&
+                  ' then there is a JS init error, please log an issue' ).
     ro_html->add( '</div>' ).
   ENDMETHOD. "render_js_error_stub
 

src/zabapgit_js_common.w3mi.data.js

@@ -87,6 +87,37 @@ function confirmInitialized() {
   debugOutput("js: OK"); // Final final confirmation :)
 }
 
+/**********************************************************
+ * Performance utils (for debugging)
+ **********************************************************/
+
+var gPerf = [];
+
+function perfOut(prefix) {
+  var totals = {};
+  for (var i = gPerf.length - 1; i >= 0; i--) {
+    if (!totals[gPerf[i].name]) totals[gPerf[i].name] = {count: 0, time: 0};
+    totals[gPerf[i].name].time  += gPerf[i].time;
+    totals[gPerf[i].name].count += 1;
+  }
+
+  var keys = Object.keys(totals);
+  for (var i = keys.length - 1; i >= 0; i--) {
+    console.log(prefix 
+      + " " + keys[i] + ": " 
+      + totals[keys[i]].time.toFixed(3) + "ms"
+      + " (" + totals[keys[i]].count.toFixed() +")");
+  }
+}
+
+function perfLog(name, startTime) {
+  gPerf.push({name: name, time: window.performance.now() - startTime});
+}
+
+function perfClear() {
+  gPerf = [];
+}
+
 /**********************************************************
  * STAGE PAGE Logic
  **********************************************************/
@@ -113,10 +144,17 @@ function StageHelper(params) {
   // Constants
   this.HIGHLIGHT_STYLE = "highlight";
   this.STATUS = {
-    "add":    "A",
+    add:    "A",
-    "remove": "R",
+    remove: "R",
-    "ignore": "I",
+    ignore: "I",
-    "reset":  "?"
+    reset:  "?",
+    isValid: function (status) { return "ARI?".indexOf(status) == -1; }
+  };
+  
+  this.TEMPLATES = {
+    cmdReset:  "<a>reset</a>",
+    cmdLocal:  "<a>add</a>",
+    cmdRemote: "<a>ignore</a><a>remove</a>"
   };
 
   this.setHooks();
@@ -154,34 +192,32 @@ StageHelper.prototype.onPageUnload = function() {
 
 // Re-store table state on entering the page
 StageHelper.prototype.onPageLoad = function() {
-
   var data = window.sessionStorage && JSON.parse(window.sessionStorage.getItem(this.pageSeed));
 
-  if (data) {
+  this.iterateStageTab(true, function (row) {
-    this.iterateStageTab(function (row) {
+    var status = data && data[row.cells[this.col.name].innerText];
-      var status = data[row.cells[this.col.name].innerText];
     this.updateRow(row, status || this.STATUS.reset);
   });
-    debugOutput("StageHelper.onPageLoad from Storage");
-
-  } else { // Render initial commands
-
-    this.iterateStageTab(function (row) {
-      this.updateRow(row, this.STATUS.reset);
-    });
-    debugOutput("StageHelper.onPageLoad initial state");
-  }
 
   this.updateMenu();
+  debugOutput("StageHelper.onPageLoad: " + ((data) ? "from Storage" : "initial state"));
 }
 
 // Table event handler, change status
 StageHelper.prototype.onTableClick = function (event) {
   var target = event.target || event.srcElement;
-  if (!target || target.tagName != "A") return;
+  if (!target) return;
 
+  if (target.tagName === "A") {
     var td = target.parentNode;
-  if (!td || ["TD","TH"].indexOf(td.tagName) == -1 || td.className != "cmd") return;
+  } else if (target.tagName === "TD") {
+    var td = target;
+    if (td.children.length === 1 && td.children[0].tagName === "A") {
+      target = td.children[0];
+    } else return;
+  } else return;
+
+  if (["TD","TH"].indexOf(td.tagName) == -1 || td.className != "cmd") return;
   
   var status    = this.STATUS[target.innerText]; // Convert anchor text to status
   var targetRow = td.parentNode;
@@ -189,7 +225,7 @@ StageHelper.prototype.onTableClick = function (event) {
   if (td.tagName === "TD") {
     this.updateRow(targetRow, status);
   } else { // TH
-    this.iterateStageTab(function (row) {
+    this.iterateStageTab(true, function (row) {
       if (row.style.display !== "none"            // Not filtered out
         && row.className === targetRow.className  // Same context as header
         ) {
@@ -208,7 +244,7 @@ StageHelper.prototype.onSearch = function (e) {
     || e.type === "keypress" && e.which === 13 ) { 
 
     this.lastSearchValue = e.target.value;
-    this.iterateStageTab(this.applyFilterToRow, e.target.value);
+    this.iterateStageTab(true, this.applyFilterToRow, e.target.value);
   }
 }
 
@@ -219,14 +255,10 @@ StageHelper.prototype.applyFilterToRow = function (row, filter) {
 
   if (filter) {
     newTxt = origTxt.replace(filter, "<mark>"+filter+"</mark>");
-    if (newTxt !== origTxt) { // fits filter
+    row.style.display = (newTxt !== origTxt) ? "" : "none";
-      row.style.display = "table-row";
-    } else {
-      row.style.display = "none";
-    }
   } else { // No filter -> just reset the value
     newTxt            = origTxt;
-    row.style.display = "table-row";
+    row.style.display = ""; // default, visible
   }
 
   if (td.firstChild.tagName === "A") {
@@ -240,7 +272,7 @@ StageHelper.prototype.applyFilterToRow = function (row, filter) {
 StageHelper.prototype.getStatusImpact = function (status) {
   if (typeof status !== "string" 
     || status.length !== 1 
-    || "ARI?".indexOf(status) == -1) {
+    || this.STATUS.isValid(status) ) {
     alert("Unknown status");
   } else {
     return (status !== this.STATUS.reset) ? 1 : 0;
@@ -252,37 +284,41 @@ StageHelper.prototype.updateRow = function (row, newStatus) {
   var oldStatus = row.cells[this.col.status].innerText;
 
   if (oldStatus !== newStatus) {
-    row.cells[this.col.status].innerText = newStatus;
+    this.updateRowStatus(row, newStatus);
-    if (newStatus === this.STATUS.reset) {
-      row.cells[this.col.status].classList.remove(this.HIGHLIGHT_STYLE);
-    } else {
-      row.cells[this.col.status].classList.add(this.HIGHLIGHT_STYLE);
-    }
     this.updateRowCommand(row, newStatus);
-  } else if (!row.cells[this.col.cmd].innerText) {
+  } else if (!row.cells[this.col.cmd].children.length) {
     this.updateRowCommand(row, newStatus); // For initial run
   }
 
   this.choiseCount += this.getStatusImpact(newStatus) - this.getStatusImpact(oldStatus);
 }
 
-// Update command cell (render set of commands)
+// Update Status cell (render set of commands)
-StageHelper.prototype.updateRowCommand = function (row, status) {
+StageHelper.prototype.updateRowStatus = function (row, status) {
-  var CMD_RESET  = "<a>reset</a>";
+  row.cells[this.col.status].innerText = status;
-  var CMD_LOCAL  = "<a>add</a>";
-  var CMD_REMOTE = "<a>ignore</a><a>remove</a>";
-
   if (status === this.STATUS.reset) {
-    row.cells[this.col.cmd].innerHTML = (row.className == "local") ? CMD_LOCAL : CMD_REMOTE;
+    row.cells[this.col.status].classList.remove(this.HIGHLIGHT_STYLE);
   } else {
-    row.cells[this.col.cmd].innerHTML = CMD_RESET;
+    row.cells[this.col.status].classList.add(this.HIGHLIGHT_STYLE);
+  }
+}
+
+// Update Command cell (render set of commands)
+StageHelper.prototype.updateRowCommand = function (row, status) {
+  var cell = row.cells[this.col.cmd];
+  if (status === this.STATUS.reset) {
+    cell.innerHTML = (row.className == "local") 
+      ? this.TEMPLATES.cmdLocal 
+      : this.TEMPLATES.cmdRemote;
+  } else {
+    cell.innerHTML = this.TEMPLATES.cmdReset;
   }
 }
 
 // Update menu items visibility
 StageHelper.prototype.updateMenu = function () {
-  this.dom.commitBtn.style.display    = (this.choiseCount > 0) ? "inline" : "none";
+  this.dom.commitBtn.style.display    = (this.choiseCount > 0) ? "" : "none";
-  this.dom.commitAllBtn.style.display = (this.choiseCount > 0) ? "none" : "inline";
+  this.dom.commitAllBtn.style.display = (this.choiseCount > 0) ? "none" : "";
   this.dom.fileCounter.innerHTML      = this.choiseCount.toString();
 }
 
@@ -294,19 +330,36 @@ StageHelper.prototype.submit = function () {
 // Extract data from the table
 StageHelper.prototype.collectData = function () {
   var data  = {};
-  this.iterateStageTab(function (row) {
+  this.iterateStageTab(false, function (row) {
     data[row.cells[this.col.name].innerText] = row.cells[this.col.status].innerText;
   });
   return data;
 }
 
-// table iteration helper
+// Table iteration helper
-StageHelper.prototype.iterateStageTab = function (cb /*, ...*/) {
+StageHelper.prototype.iterateStageTab = function (changeMode, cb /*, ...*/) {
-  for (var b = 0, bN = this.dom.stageTab.tBodies.length; b < bN; b++) {
+  
-    var tbody = this.dom.stageTab.tBodies[b];
+  var restArgs = Array.prototype.slice.call(arguments, 2);
+  var table    = this.dom.stageTab;
+
+  if (changeMode) {
+    var scrollOffset = window.pageYOffset;
+    this.dom.stageTab.style.display = "none";
+    // var stageTabParent = this.dom.stageTab.parentNode;
+    // table = stageTabParent.removeChild(this.dom.stageTab);
+  }
+
+  for (var b = 0, bN = table.tBodies.length; b < bN; b++) {
+    var tbody = table.tBodies[b];
     for (var r = 0, rN = tbody.rows.length; r < rN; r++) {
-      args = [tbody.rows[r]].concat(Array.prototype.slice.call(arguments, 1));
+      args = [tbody.rows[r]].concat(restArgs);
       cb.apply(this, args); // callback
     }
   }
+
+  if (changeMode) {
+    this.dom.stageTab.style.display = "";
+    // stageTabParent.appendChild(table);
+    window.scrollTo(0, scrollOffset);
+  }
 }

src/zabapgit_page_stage.prog.abap

@@ -31,21 +31,25 @@ CLASS lcl_gui_page_stage DEFINITION FINAL INHERITING FROM lcl_gui_page.
     METHODS:
       render_list
         RETURNING VALUE(ro_html) TYPE REF TO lcl_html,
+
       render_file
         IMPORTING iv_context     TYPE string
                   is_file        TYPE ty_file
                   is_item        TYPE ty_item OPTIONAL
         RETURNING VALUE(ro_html) TYPE REF TO lcl_html,
-      render_menu
+
+      render_actions
         RETURNING VALUE(ro_html) TYPE REF TO lcl_html,
+
       read_last_changed_by
         IMPORTING is_file        TYPE ty_file
-        RETURNING VALUE(rv_user) TYPE xubname.
+        RETURNING VALUE(rv_user) TYPE xubname,
 
-    METHODS process_stage_list
+      process_stage_list
         IMPORTING it_postdata TYPE cnht_post_data_tab
-      RAISING   lcx_exception.
+        RAISING   lcx_exception,
-    METHODS build_menu
+
+      build_menu
         RETURNING VALUE(ro_menu) TYPE REF TO lcl_html_toolbar.
 
 ENDCLASS.
@@ -162,7 +166,7 @@ CLASS lcl_gui_page_stage IMPLEMENTATION.
 
     CREATE OBJECT ro_html.
 
-    ro_html->add( '<table id="stageTab" class="stage_tab">' ).
+    ro_html->add( '<table id="stageTab" class="stage_tab w100">' ).
 
     " Local changes
     LOOP AT ms_files-local ASSIGNING <ls_local>.
@@ -172,7 +176,9 @@ CLASS lcl_gui_page_stage IMPLEMENTATION.
         ro_html->add( '<th>Files to add (click to see diff)</th>' ).
         ro_html->add( '<th>Changed by</th>' ).
         ro_html->add( '<th></th>' ). " Status
-        ro_html->add('<th class="cmd">&#x2193;<a>add</a>/<a>reset</a>&#x2193;</th>' ).
+        ro_html->add( '<th class="cmd">' ).
+        ro_html->add( '<a>add</a>&#x2193; <a>reset</a>&#x2193;' ).
+        ro_html->add( '</th>' ).
         ro_html->add( '</tr></thead>' ).
         ro_html->add( '<tbody>' ).
       ENDAT.
@@ -194,8 +200,9 @@ CLASS lcl_gui_page_stage IMPLEMENTATION.
         ro_html->add( '<th></th>' ). " Type
         ro_html->add( '<th colspan="2">Files to remove or non-code</th>' ).
         ro_html->add( '<th></th>' ). " Status
-        ro_html->add( '<th class="cmd">' &&
+        ro_html->add( '<th class="cmd">' ).
-                      '&#x2193;<a>ignore</a><a>remove</a><a>reset</a>&#x2193;</th>' ).
+        ro_html->add( '<a>ignore</a>&#x2193; <a>remove</a>&#x2193; <a>reset</a>&#x2193;' ).
+        ro_html->add( '</th>' ).
         ro_html->add( '</tr></thead>' ).
         ro_html->add( '<tbody>' ).
       ENDAT.
@@ -252,38 +259,52 @@ CLASS lcl_gui_page_stage IMPLEMENTATION.
     ro_html->add( '<div class="repo">' ).
     ro_html->add( lcl_gui_chunk_lib=>render_repo_top( mo_repo ) ).
     ro_html->add( lcl_gui_chunk_lib=>render_js_error_banner( ) ).
-    ro_html->add( render_menu( ) ).
+
+    ro_html->add( '<div class="stage-container">' ).
+    ro_html->add( render_actions( ) ).
     ro_html->add( render_list( ) ).
     ro_html->add( '</div>' ).
 
+    ro_html->add( '</div>' ).
+
   ENDMETHOD.      "render_content
 
-  METHOD render_menu.
+  METHOD render_actions.
 
-    DATA lv_local_count TYPE i.
+    DATA: lv_local_count TYPE i,
+          lv_add_all_txt TYPE string.
 
     CREATE OBJECT ro_html.
     lv_local_count = lines( ms_files-local ).
+    IF lv_local_count > 0.
+      lv_add_all_txt = |Add all and commit ({ lv_local_count })|.
+      " Otherwise empty, but the element (id) is preserved for JS
+    ENDIF.
 
-    ro_html->add( '<div class="paddings">' ).
+    ro_html->add( '<table class="w100 margin-v5"><tr>' ).
+
+    " Action buttons
+    ro_html->add( '<td class="indent5em">' ).
     ro_html->add_a( iv_act   = 'errorStub(event)' " Will be reinit by JS
                     iv_typ   = gc_action_type-onclick
                     iv_id    = 'commitButton'
                     iv_style = 'display: none'
                     iv_txt   = 'Commit (<span id="fileCounter"></span>)'
                     iv_opt   = gc_html_opt-strong ) ##NO_TEXT.
-    IF lv_local_count > 0.
     ro_html->add_a( iv_act = |{ c_action-stage_all }|
                     iv_id  = 'commitAllButton'
-                      iv_txt = |Add all and commit ({ lv_local_count })| ) ##NO_TEXT.
+                    iv_txt = lv_add_all_txt ) ##NO_TEXT.
-    ENDIF.
+    ro_html->add( '</td>' ).
-    ro_html->add( '</div>' ).
 
-    ro_html->add( '<div>' ).
+    " Filter bar
-    ro_html->add( '<input id="objectSearch" type="search" placeholder="Filter objects">' ).
+    ro_html->add( '<td class="right">' ).
-    ro_html->add( '</div>' ).
+    ro_html->add( '<input class="stage-filter" id="objectSearch"' &&
+                  ' type="search" placeholder="Filter objects">' ).
+    ro_html->add( '</td>' ).
 
-  ENDMETHOD.      "render_menu
+    ro_html->add( '</tr></table>' ).
+
+  ENDMETHOD.      "render_actions
 
   METHOD scripts.