From 9f784017b826f75bd9f3f5f7c037233c4000c33e Mon Sep 17 00:00:00 2001
From: Brian Hann <emailc0bra@gmail.com>
Date: Wed, 8 May 2013 13:24:01 -0500
Subject: [PATCH] Fixes #399, making cell editing work again

This is an issue with how src/ng-input.js is handling the value of the ng-input directive. The current default template contains an $eval, which means that when ng-input.js runs a $parse on it, it gets the evaluated value of the cell itself. For a string, (which is an object) this ends up not actually modifying the cell contents but at least it doesn't fire an exception. For an integer primitive you would end up with an undefined setter and a noop getter, which would throw exceptions when you try to use undefined as a function to set the cell value.

The fix, as I see it, is to use a regex to strip out the $eval statement from the ng-input in the editable cell template.

I've also cleaned up ng-input.js, added some comments, and added handling
for when the enter key is used to modify a cell's contents
---
 src/directives/ng-input.js  | 50 +++++++++++++++++++++++++------------
 test/unit/directivesSpec.js | 37 +++++++++++++++++++++------
 2 files changed, 64 insertions(+), 23 deletions(-)

diff --git a/src/directives/ng-input.js b/src/directives/ng-input.js
index eb836cd69c..5511a4759a 100644
--- a/src/directives/ng-input.js
+++ b/src/directives/ng-input.js
@@ -1,35 +1,53 @@
 ngGridDirectives.directive('ngInput',['$parse', function($parse) {
-    return function ($scope, elm, attrs) {
-        var getter = $parse($scope.$eval(attrs.ngInput));
+  return function ($scope, elm, attrs) {
+    
+    // Strip the $eval off of the ng-input tag, we want WHERE to put the edited values, not the actual edited/displayed value itself
+    var inputter = attrs.ngInput.replace(/.+?\((.+?)\)/, '$1');
+
+    var evaled = $scope.$eval(inputter);
+    var getter = $parse(evaled);
 		var setter = getter.assign;
-		var oldCellValue = getter($scope.row.entity);
+		var oldCellValue = getter($scope);
+
 		elm.val(oldCellValue);
-        elm.bind('keyup', function() {
-            var newVal = elm.val();
-            if (!$scope.$root.$$phase) {
-                $scope.$apply(function(){setter($scope.row.entity,newVal); });
-            }
-        });
+
+    elm.bind('keyup', function() {
+        var newVal = elm.val();
+        // dump(newVal);
+        if (!$scope.$root.$$phase) {
+          $scope.$apply(function(){
+            setter($scope, newVal);
+          });
+        }
+    });
+
 		elm.bind('keydown', function(evt){
 			switch(evt.keyCode){
-				case 37:
-				case 38:
-				case 39:
-				case 40:
+				case 37: // Left arrow
+				case 38: // Up arrow
+				case 39: // Right arrow
+				case 40: // Down arrow
 					evt.stopPropagation();
 					break;
-				case 27:
+				case 27: // Esc (reset to old value)
 					if (!$scope.$root.$$phase) {
 						$scope.$apply(function(){
-							setter($scope.row.entity,oldCellValue);
+							setter($scope, oldCellValue);
 							elm.val(oldCellValue);
 							elm.blur();
 						});
 					}
+        case 13: // Enter (apply new value)
+          if (!$scope.$root.$$phase) {
+            $scope.$apply(function(){
+              setter($scope, elm.val());
+              elm.blur();
+            });
+          }
 				default:
 					break;
 			}
 			return true;
 		});
-    };
+  };
 }]);
\ No newline at end of file
diff --git a/test/unit/directivesSpec.js b/test/unit/directivesSpec.js
index 8ab6b658de..52d69a0dc1 100644
--- a/test/unit/directivesSpec.js
+++ b/test/unit/directivesSpec.js
@@ -126,19 +126,22 @@ describe('directives', function () {
             });
             
             describe('column', function () {
-                describe('enableCellEdit option', function () {
-                    it('should allow colDef.enableCellEdit to override config.enableCellEdit', inject(function ($rootScope, $compile) {
+                describe('cell editing', function () {
+                    var elm, element, $scope;
+
+                    beforeEach(inject(function ($rootScope, $compile) {
+                        $scope = $rootScope;
                         elm = angular.element(
                             '<div ng-grid="gridOptions" style="width: 1000px; height: 1000px"></div>'
                         );
 
-                        $rootScope.myData = [{name: "Moroni", age: 50},
+                        $scope.myData = [{name: "Moroni", age: 'test' },
                                         {name: "Tiancum", age: 43},
                                         {name: "Jacob", age: 27},
                                         {name: "Nephi", age: 29},
                                         {name: "Enos", age: 34}];
 
-                        $rootScope.gridOptions = {
+                        $scope.gridOptions = {
                             data: 'myData',
                             enableCellEdit: true,
                             columnDefs: [
@@ -147,9 +150,11 @@ describe('directives', function () {
                             ]
                         };
 
-                        var element = $compile(elm)($rootScope);
-                        $rootScope.$digest();
+                        element = $compile(elm)($scope);
+                        $scope.$digest();
+                    }));
 
+                    it('should allow colDef.enableCellEdit to override config.enableCellEdit', function() {
                         var cell = element.find('.ngRow:eq(0) .ngCell.col0 [ng-dblclick]');
 
                         // Have to use jasmine's async testing helpers to get around the setTimeout(..., 0) in ng-cell-has-focus.editCell()
@@ -160,7 +165,25 @@ describe('directives', function () {
                         runs(function() {
                             expect(cell.find('input').length).toEqual(0);
                         });
-                    }));
+                    });
+
+                    it('should not throw an exception when the column contains a non-string', function() {
+                        var cell = element.find('.ngRow:eq(0) .ngCell.col1 [ng-dblclick]');
+
+                        runs(function() {
+                            browserTrigger(cell, 'dblclick');
+                        });
+                        waits(200);
+                        runs(function() {
+                            expect(function(){
+                                // Trigger the input handler
+                                var input = cell.find('input');
+                                input.triggerHandler('keyup');
+                            }).not.toThrow();
+                        });
+                    });
+
+                    // TODO: add a test for enter key modifying the inputted contents
                 });
 
                 describe('displayName option', function() {