LP#1635386 Clarify and simplify row highlighting code
authorDan Wells <dbw2@calvin.edu>
Fri, 23 Mar 2018 21:21:36 +0000 (17:21 -0400)
committerDan Wells <dbw2@calvin.edu>
Thu, 28 Jun 2018 18:21:06 +0000 (14:21 -0400)
The existing code had a few functional problems for me.  In the process
of fixing them, I decided it best to also apply a number of small
refactorings.  Here is a list of changes in rough order of significance:

-Made the new CSS classes test and apply once per row rather than once
per cell

-Fixed the test case for overdues: we now just look for no checkin-time
+ no lost/long-overdue stop-fines (matches XUL test)

-Made the color and icon tests totally consistent, and also simplified
where possible

-Made the widened configuration header style (to accommodate status
column) functional again (it was using old 'statusicon' class)

-Made row highlight colors '!important' to avoid bad interactions with
alternating row color styles

-Changed status-cell (and statusCell) to status-column (and
statusColumn) for a little extra clarity

-Changed nested function name from 'rowClass()' to 'apply()' for greater
code distinction (i.e. avoid 'rowClass.rowClass()' calls, and instead
have 'rowClass.apply()')

-Removed some unused variable assignments from grid.js

-Cleaned up a few field attributes for necessity and consistency

Signed-off-by: Dan Wells <dbw2@calvin.edu>
Signed-off-by: Kathy Lussier <klussier@masslnc.org>
Signed-off-by: Dawn Dale <ddale@georgialibraries.org>

Open-ILS/src/templates/staff/circ/patron/t_bills_list.tt2
Open-ILS/src/templates/staff/css/style.css.tt2
Open-ILS/src/templates/staff/share/t_autogrid.tt2
Open-ILS/web/js/ui/default/staff/circ/patron/bills.js
Open-ILS/web/js/ui/default/staff/services/grid.js

index ad1db19..c843bbb 100644 (file)
@@ -8,7 +8,7 @@
   persist-key="circ.patron.bills"
   dateformat="{{$root.egDateAndTimeFormat}}"
   row-class="colorizeBillsList"
-  status-cell="statusIconColumn">
+  status-column="statusIconColumn">
 
   <eg-grid-menu-item label="[% l('Bill Patron') %]" 
     handler="showBillDialog"></eg-grid-menu-item>
@@ -50,7 +50,7 @@
   <eg-grid-action label="[% l('Full Details') %]" 
     handler="showFullDetails"></eg-grid-action>
 
-  <eg-grid-field label="[% l('Balance Owed') %]" path='summary.balance_owed' required></eg-grid-field>
+  <eg-grid-field label="[% l('Balance Owed') %]" path='summary.balance_owed'></eg-grid-field>
   <eg-grid-field label="[% l('Bill #') %]" path='id'></eg-grid-field>
   <eg-grid-field label="[% l('Start') %]" path='xact_start' datatype="timestamp"></eg-grid-field>
   <eg-grid-field label="[% l('Total Billed') %]" path='summary.total_owed'></eg-grid-field>
   <eg-grid-field path='circulation.circ_lib' required hidden></eg-grid-field>
   <eg-grid-field path='circulation.duration' required hidden></eg-grid-field>
   <eg-grid-field path='circulation.due_date' dateonlyinterval="circulation.duration" datecontext="circulation.circ_lib" required hidden></eg-grid-field>
-  <eg-grid-field label="[% l('Stop Fines') %]" path="circulation.stop_fines" hidden required>  </eg-grid-field>
-  <eg-grid-field path="circulation.checkin_time" hidden required></eg-grid-field>
+  <eg-grid-field label="[% l('Stop Fines') %]" path="circulation.stop_fines" required hidden></eg-grid-field>
+  <eg-grid-field path="circulation.checkin_time" required hidden></eg-grid-field>
   <eg-grid-field path='circulation.*' hidden> </eg-grid-field>
   <eg-grid-field label="[% l('Checkout / Renewal Library') %]"
     path='circulation.circ_lib.shortname' required hidden> </eg-grid-field>
index 6ca02ff..94e4e86 100644 (file)
@@ -301,17 +301,18 @@ table.list tr.selected td { /* deprecated? */
   text-align: center;
 }
 
-.eg-grid-cell-stock-status {
+/* the conf header must be twice the stock flex */
+.eg-grid-cell-conf-header {
   width: 4.4em;
-  text-align: center;
+  font-weight: bold;
 }
 
-/* the conf header must be ~four times the stock flex */
-.eg-grid-cell-conf-header {
+.eg-grid-cell-stock-status {
   width: 4.4em;
-  font-weight: bold;
+  text-align: center;
 }
 
+/* the conf header must be 4x the stock width when status is present */
 .eg-grid-cell-conf-header-status {
   width: 8.8em;
   font-weight: bold;
@@ -347,28 +348,28 @@ table.list tr.selected td { /* deprecated? */
 /* patron bill row-highlighting */
 .red-row-highlight {
   color: #FFF;
-  background-color: #EB0000;
+  background-color: #EB0000 !important;
 }
 .red-row-highlight a:link, .dark-red-row-highlight a:link {
   color: #B8ECFF;
 }
 .orange-row-highlight {
   color: #000;
-  background-color: #FFE1A8;
+  background-color: #FFE1A8 !important;
 }
 .dark-red-row-highlight {
   color: #FFF;
-  background-color: #800000;
+  background-color: #800000 !important;
 
 }
-.eg-grid-row-selected .red-row-highlight {
-  background-color: #CF0000;
+.eg-grid-row-selected.red-row-highlight {
+  background-color: #CF0000 !important;
 }
-.eg-grid-row-selected .orange-row-highlight {
-  background-color: #FFCF75;
+.eg-grid-row-selected.orange-row-highlight {
+  background-color: #FFCF75 !important;
 }
-.eg-grid-row-selected .dark-red-row-highlight {
-  background-color: #5C0000;
+.eg-grid-row-selected.dark-red-row-highlight {
+  background-color: #5C0000 !important;
 }
 
 .eg-grid-cell-content::selection {
index 21d602b..58b3534 100644 (file)
           type='checkbox' ng-model="selectAll"/> 
       </div>
     </div>
-    <div class="eg-grid-cell eg-grid-cell-stock-status" ng-show="statusCell.isEnabled">
+    <div class="eg-grid-cell eg-grid-cell-stock-status" ng-show="statusColumn.isEnabled">
       <div title="[% l('Status Icon Column') %]">[% l('Status') %]</div>
     </div>
     <div class="eg-grid-cell"
   <!-- Inline grid configuration row -->
   <div class="eg-grid-row eg-grid-conf-row" ng-show="showGridConf">
     <div class="eg-grid-cell"
-      ng-class="statusicon.isEnabled ? 'eg-grid-cell-conf-header-status' : 'eg-grid-cell-conf-header'">
+      ng-class="statusColumn.isEnabled ? 'eg-grid-cell-conf-header-status' : 'eg-grid-cell-conf-header'">
       <div class="eg-grid-conf-cell-entry">[% l('Expand') %]</div>
       <div class="eg-grid-conf-cell-entry">[% l('Shrink') %]</div>
     </div>
         id="eg-grid-row-{{$index + 1}}"
         ng-repeat="item in items"
         ng-show="items.length > 0"
-        ng-class="{'eg-grid-row-selected' : selected[indexValue(item)]}">
+        ng-class="[{'eg-grid-row-selected' : selected[indexValue(item)]}, rowClass.apply(item)]">
       <div class="eg-grid-cell eg-grid-cell-stock" ng-show="showIndex"
-        ng-click="handleRowClick($event, item)" title="[% l('Row Index') %]"
-          ng-class="rowClass.rowClass(item)">
+        ng-click="handleRowClick($event, item)" title="[% l('Row Index') %]">
         <a href ng-show="gridControls.activateItem" 
           ng-click="gridControls.activateItem(item)" style="font-weight:bold">
           {{$index + offset() + 1}}
         </a>
         <div ng-hide="gridControls.activateItem">{{$index + offset() + 1}}</div>
       </div>
-      <div class="eg-grid-cell eg-grid-cell-stock" ng-show="canMultiSelect"
-          ng-class="rowClass.rowClass(item)">
+      <div class="eg-grid-cell eg-grid-cell-stock" ng-show="canMultiSelect">
         <!-- ng-click=handleRowClick here has unintended 
              consequences and is unnecessary, avoid it -->
         <div>
             ng-model="selected[indexValue(item)]"/>
         </div>
       </div>
-      <div class="eg-grid-cell eg-grid-cell-stock-status" ng-show="statusCell.isEnabled"
-          ng-class="rowClass.rowClass(item)">
-          <span ng-bind-html="statusCell.template(item)"></span>
+      <div class="eg-grid-cell eg-grid-cell-stock-status" ng-show="statusColumn.isEnabled">
+          <span ng-bind-html="statusColumn.template(item)"></span>
       </div>
       <div class="eg-grid-cell eg-grid-cell-content"
           ng-click="handleRowClick($event, item)"
           ng-dblclick="gridControls.activateItem(item)"
           ng-repeat="col in columns"
           style="text-align:{{col.align}}; flex:{{col.flex}}"
-          ng-show="col.visible"
-          ng-class="rowClass.rowClass(item)">
+          ng-show="col.visible">
 
           <!-- if the cell comes with its own template,
                translate that content into HTML and insert it here -->
index a84ab2c..bd1e9be 100644 (file)
@@ -222,14 +222,13 @@ function($scope , $q , $routeParams , egCore , egConfirmDialog , $location,
     // -------------
     // Apply coloring to rows based on fines stop reason
     $scope.colorizeBillsList = {
-        rowClass: function(item) {
+        apply: function(item) {
             if (!item['circulation.checkin_time']) {
                 if (item['circulation.stop_fines'] == 'LOST') {
                     return 'dark-red-row-highlight';
                 } else if (item['circulation.stop_fines'] == 'LONGOVERDUE') {
                     return 'red-row-highlight';
-                } else if (item['circulation.due_date'] &&  // Still checked out - need feedback on this approach, feel like there's a better way
-                          !item['circulation.stop_fines']) {
+                } else {
                     return 'orange-row-highlight';
                 }
             }
@@ -240,29 +239,17 @@ function($scope , $q , $routeParams , egCore , egConfirmDialog , $location,
     $scope.statusIconColumn = {
         isEnabled: true,
         template: function(item) {
-            var template = "";
-            var icons = [];
-            var now = new Date();
-
-            if (item['circulation.due_date'] &&
-                !item['circulation.checkin_time']) {
-                var due_date = new Date(item['circulation.due_date']);
-
-                if (item['circulation.stop_fines'] &&
-                    item['circulation.stop_fines'] == "LOST") {
-                    icons.push('glyphicon-question-sign');
-                } else if (item['circulation.stop_fines'] &&
-                    item['circulation.stop_fines'] == "LONGOVERDUE") {
-                    icons.push('glyphicon-exclamation-sign');
-                } else if (now >= due_date) {
-                    icons.push('glyphicon-time');
+            var icon = '';
+            if (!item['circulation.checkin_time']) {
+                if (item['circulation.stop_fines'] == "LOST") {
+                    icon = 'glyphicon-question-sign';
+                } else if (item['circulation.stop_fines'] == "LONGOVERDUE") {
+                    icon = 'glyphicon-exclamation-sign';
+                } else {
+                    icon = 'glyphicon-time';
                 }
             }
-
-            angular.forEach(icons, function(icon) {
-                template = template + "<i class='glyphicon " + icon + "'></i>"
-            });
-            return template;
+            return "<i class='glyphicon " + icon + "'></i>"
         }
     }
 
index 71e271c..c774c7f 100644 (file)
@@ -60,7 +60,7 @@ angular.module('egGridMod',
 
             // optional: object that enables status icon field and contains
             //    function to handle what status icons should exist and why.
-            statusCell : '=',
+            statusColumn : '=',
 
             // optional primary grid label
             mainLabel : '@',
@@ -118,9 +118,6 @@ angular.module('egGridMod',
             scope.handleAutoFields();
             scope.collect();
 
-            var statusCell = scope.statusCell;
-            var rowClass = scope.rowClass;
-
             scope.grid_element = element;
             $(element)
                 .find('.eg-grid-content-body')