Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Major bug in editable ng-grid cells. #399

Closed
kelf opened this issue May 8, 2013 · 3 comments
Closed

Major bug in editable ng-grid cells. #399

kelf opened this issue May 8, 2013 · 3 comments
Assignees
Milestone

Comments

@kelf
Copy link

kelf commented May 8, 2013

If I set an editable cell's data to a number, a string CONTAINING a number, undefined, a regex, or an object I get an exception.

Reproduced here. http://plnkr.co/edit/PAHeSd?p=preview

Try editing a value in the age column.

@c0bra
Copy link
Contributor

c0bra commented May 8, 2013

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.

c0bra added a commit that referenced this issue May 8, 2013
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
@ghost ghost assigned c0bra May 8, 2013
@c0bra
Copy link
Contributor

c0bra commented May 8, 2013

Closed in 2.0.6 branch

@c0bra c0bra closed this as completed May 8, 2013
This was referenced May 9, 2013
@sayeed-anjum
Copy link

I have a doubt if we really need to fix this here since I believe the regression happened when fixing issue #337 where $eval was added around the COL_FIELD which is perhaps not needed.

Idea is to fix the root cause instead of fixing a symptom.

We could revert back to:

html = html.replace(EDITABLE_CELL_TEMPLATE, $scope.col.editableCellTemplate.replace(COL_FIELD, "col.field"));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants