Grid Select Improvements (#138)

* Remove NaN values form number fields in grids Closes #119 * Fix [other] option bug in grid selects Related to #75 * Prevent entering an existing option in the "Other" dialog Closes #75 * switch to agrc/grunt-contrib-jasmine temporarily until gruntjs/grunt-contrib-jasmine#269 is merged Other misc test cleanup * Upgrade permissions on travis to help with Chrome Headless travis-ci/travis-ci#8836 * fix tests
agrc · May 17, 2018 · 29ba8f0 · 29ba8f0
1 parent 82f2e60
commit 29ba8f0
Show file tree

Hide file tree

Showing 18 changed files with 1,610 additions and 1,555 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -12,4 +12,6 @@ notifications:
     email:
         on_success: change
         on_failure: change
-sudo: false
+sudo: required
+addons:
+  chrome: stable
diff --git a/Gruntfile.js b/Gruntfile.js
@@ -143,17 +143,17 @@ module.exports = function (grunt) {
             main: {
                 options: {
                     specs: ['src/app/**/Spec*.js'],
+                    // specs: ['src/app/tests/spec/SpecMoreInfoDialog.js'],
                     vendor: [
                         'src/jasmine-favicon-reporter/vendor/favico.js',
                         'src/jasmine-favicon-reporter/jasmine-favicon-reporter.js',
                         'src/jasmine-jsreporter/jasmine-jsreporter.js',
                         'src/app/tests/jasmineTestBootstrap.js',
                         'src/dojo/dojo.js',
                         'src/app/packages.js',
-                        'src/app/tests/jsReporterSanitizer.js',
                         'src/app/tests/jasmineAMDErrorChecking.js',
                         'src/jquery/dist/jquery.js',
-                        'node_modules/es6-object-assign/dist/object-assign-auto.min.js'
+                        'src/bootstrap/dist/js/bootstrap.js'
                     ],
                     host: 'http://localhost:8000'
                 }

diff --git a/_src/app/Domains.js b/_src/app/Domains.js
@@ -4,16 +4,14 @@ define([
     'app/config',
     'app/OtherOptionHandler',
 
-    'dojo/dom-construct',
-    'dojo/_base/array'
+    'dojo/dom-construct'
 ], function (
     agrcDomains,
 
     config,
     OtherOptionHandler,
 
-    domConstruct,
-    array
+    domConstruct
 ) {
     // summary:
     //      Used to feed options to comboboxes
@@ -67,29 +65,20 @@ define([
             //      fires when a select is changed to the "other" option
             console.log('app/Domains:onOtherSelected', arguments);
 
-            var existingOptions = [];
-
-            array.forEach(select.children, function (option) {
-                if (option.value !== '' && option.value !== this.otherTxt) {
-                    existingOptions.push({
-                        code: option.value,
-                        name: option.innerHTML
-                    });
-                }
-            }, this);
+            var existingOptions = Array.from(select.children).map(option => option.value);
 
             var ooh = new OtherOptionHandler({
-                existingOptions: existingOptions,
+                existingOptions,
                 otherTxt: this.otherTxt
             }, domConstruct.create('div', null, document.body));
             ooh.startup();
 
-            ooh.on('add-new-value', function (newValue) {
+            ooh.on('add-new-value', function (event) {
                 domConstruct.create('option', {
-                    innerHTML: newValue.name,
-                    value: newValue.code
+                    innerHTML: event.code,
+                    value: event.code
                 }, select);
-                select.value = newValue.code;
+                select.value = event.code;
 
                 $(select).combobox('refresh');
             });

diff --git a/_src/app/OtherOptionHandler.js b/_src/app/OtherOptionHandler.js
@@ -47,9 +47,8 @@ define([
             console.log('app/OtherOptionHandler:postCreate', arguments);
 
             this.existingOptions.forEach(function (option) {
-                if (option.code !== '' && option.code !== this.otherTxt) {
-                    var txt = option.name + ' (' + option.code + ')';
-                    domConstruct.create('li', {innerHTML: txt}, this.existingOptionsList);
+                if (option !== '' && option !== this.otherTxt) {
+                    domConstruct.create('li', {innerHTML: option}, this.existingOptionsList);
                 }
             }, this);
         },
@@ -71,23 +70,27 @@ define([
 
             $(this.modal).modal('hide');
 
-            this.emit('add-new-value', {
-                name: this.descTxt.value,
-                code: this.codeTxt.value
-            });
+            this.emit('add-new-value', { code: this.codeTxt.value });
         },
         onCancel: function () {
             // summary:
             //      user has clicked the close button
             // param or return
             console.log('app/OtherOptionHandler:onCancel', arguments);
         },
-        onTxtChange: function () {
+        onTxtChange: function (event) {
             // summary:
             //      sets the disabled state of the submit button
+            // event: Event Object
             console.log('app/OtherOptionHandler:onTxtChange', arguments);
 
-            this.submitBtn.disabled = !(this.codeTxt.value.length > 0 && this.descTxt.value.length > 0);
+            const value = this.codeTxt.value.toUpperCase();
+            this.submitBtn.disabled = !value || !(value.length > 0) ||
+                this.existingOptions.map(v => v.toUpperCase()).includes(value);
+
+            if (!this.submitBtn.disabled && event.key === 'Enter') {
+                this.onSubmit();
+            }
         },
         destroyRecursive: function () {
             // summary:

diff --git a/_src/app/_GridMixin.js b/_src/app/_GridMixin.js
@@ -1,12 +1,16 @@
 define([
     './catch/NoFishException',
+
     'dgrid/Editor',
     'dgrid/Keyboard',
     'dgrid/OnDemandGrid',
     'dgrid/Selection',
 
+    'dijit/form/NumberSpinner',
+
     'dojo/keys',
     'dojo/on',
+    'dojo/topic',
     'dojo/_base/array',
     'dojo/_base/declare',
     'dojo/_base/lang',
@@ -15,13 +19,17 @@ define([
     'dstore/Trackable'
 ], function (
     NoFishException,
+
     Editor,
     Keyboard,
     DGrid,
     Selection,
 
+    NumberSpinner,
+
     keys,
     on,
+    topic,
     array,
     declare,
     lang,
@@ -32,6 +40,16 @@ define([
     // summary:
     //      Mixin to add dgrid to a widget.
     return declare(null, {
+        // this is to remove the default NaN value if there is an empty string
+        // it's used as a column editor in classes that use this mixin
+        NewNumberSpinner: declare([NumberSpinner], {
+            value: null,
+            _getValueAttr() {
+                const inheritedValue = this.inherited(arguments);
+
+                return isNaN(inheritedValue) ? null : inheritedValue;
+            }
+        }),
 
         // grid: DGrid
         grid: null,
@@ -63,6 +81,19 @@ define([
             on(this.grid, 'dgrid-deselect', lang.hitch(this, this.onRowDeselected));
 
             this.setGridData([]);
+
+            this.own(
+                topic.subscribe(`refocus_${this.id}`, (columnIndex) => {
+                    for (var id in this.grid.selection) {
+                        if (this.grid.selection.hasOwnProperty(id)) {
+                            // columnIndex needs to be a string
+                            const cell = this.grid.cell(id, columnIndex + '');
+                            console.log(cell);
+                            this.grid.edit(cell);
+                        }
+                    }
+                })
+            );
         },
         onRowSelected: function (evt) {
             // summary:

diff --git a/_src/app/_InProgressCacheMixin.js b/_src/app/_InProgressCacheMixin.js
@@ -60,7 +60,10 @@ define([
                 }
             });
 
-            this.hydrateWithInProgressData();
+            // don't run this in jasmine tests
+            if (!window.jasmine) {
+                this.hydrateWithInProgressData();
+            }
 
             this.inherited(arguments);
         },

diff --git a/_src/app/catch/Catch.js b/_src/app/catch/Catch.js
@@ -8,7 +8,6 @@ define([
     'app/GridTab',
     'app/_GridMixin',
 
-    'dijit/form/NumberSpinner',
     'dijit/_TemplatedMixin',
     'dijit/_WidgetBase',
     'dijit/_WidgetsInTemplateMixin',
@@ -43,7 +42,6 @@ define([
     GridTab,
     _GridMixin,
 
-    NumberSpinner,
     _TemplatedMixin,
     _WidgetBase,
     _WidgetsInTemplateMixin,
@@ -158,7 +156,8 @@ define([
                     editorArgs: {
                         domainFieldName: fn.SPECIES_CODE,
                         domainLayerUrl: config.urls.fishFeatureService,
-                        grid: this.grid
+                        parentId: this.id,
+                        columnIndex: 5
                     }
                 }, {
                     autoSave: true,
@@ -170,13 +169,14 @@ define([
                     editorArgs: {
                         domainFieldName: fn.LENGTH_TYPE,
                         domainLayerUrl: config.urls.fishFeatureService,
-                        grid: this.grid
+                        parentId: this.id,
+                        columnIndex: 6
                     }
                 }, {
                     autoSave: true,
                     label: 'Length (millimeters)',
                     field: fn.LENGTH,
-                    editor: NumberSpinner,
+                    editor: this.NewNumberSpinner,
                     sortable: false,
                     autoSelect: true,
                     editOn: 'focus',
@@ -189,7 +189,7 @@ define([
                     autoSave: true,
                     label: 'Weight (grams)',
                     field: fn.WEIGHT,
-                    editor: NumberSpinner,
+                    editor: this.NewNumberSpinner,
                     sortable: false,
                     autoSelect: true,
                     editOn: 'focus',
@@ -231,31 +231,23 @@ define([
                 $(that.batchBtn).popover('hide');
             });
 
-            this.own(
-                topic.subscribe('refocus', function () {
-                    for (var id in that.grid.selection) {
-                        if (that.grid.selection.hasOwnProperty(id)) {
-                            that.grid.edit(that.grid.cell(id, '6'));
+            if (!window.jasmine) {
+                localforage.getItem(this.cacheId).then(function (inProgressData) {
+                    if (inProgressData) {
+                        if (inProgressData.gridData) {
+                            that.setGridData(inProgressData.gridData);
+                            that.grid.refresh();
                         }
-                    }
-                })
-            );
-
-            localforage.getItem(this.cacheId).then(function (inProgressData) {
-                if (inProgressData) {
-                    if (inProgressData.gridData) {
-                        that.setGridData(inProgressData.gridData);
-                        that.grid.refresh();
-                    }
 
-                    if (inProgressData.numPasses > 1) {
-                        for (var i = 1; i < inProgressData.numPasses; i++) {
-                            that.gridTab.addTab(true);
+                        if (inProgressData.numPasses > 1) {
+                            for (var i = 1; i < inProgressData.numPasses; i++) {
+                                that.gridTab.addTab(true);
+                            }
                         }
                     }
-                }
-                that.store.on('add, update, delete', lang.hitch(that, 'cacheInProgressData'));
-            });
+                    that.store.on('add, update, delete', lang.hitch(that, 'cacheInProgressData'));
+                });
+            }
 
             this.wireBatchFormEvents();