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

ui-grid-row-edit bug #1694

Closed
6gsaifulislam opened this issue Oct 3, 2014 · 7 comments · Fixed by #1705
Closed

ui-grid-row-edit bug #1694

6gsaifulislam opened this issue Oct 3, 2014 · 7 comments · Fixed by #1705
Assignees
Milestone

Comments

@6gsaifulislam
Copy link

If I include ui-grid-row-edit, i.e. as follows:

the grid columns names are created automatically without setting grid columns in code and cannot be changed i.e. added or removed.

@PaulL1
Copy link
Contributor

PaulL1 commented Oct 3, 2014

I think there is some content missing from your issue.

From what I can join the dots, I think you're saying that if you don't specify columns via columnDefs, then the grid automatically creates columns based on the data. This is expected behaviour, and is a feature and not a bug.

The columns should be able to be changed via gridOptions.columnDefs if you wanted to do so. A plunkr would help to explain what your concern is.

@6gsaifulislam
Copy link
Author

Hi, playing around with it and I found that the problem was corrected in the latest version of the unstable release (I was using a copy which was 30 hours old!).

What I am trying to do is replace the existing data with new data which has different column layout. However it tends to leave columns in which should have been removed.
I have got it displayed in a plunker at: http://plnkr.co/edit/4CeFLnHBh7gza5KmXTlE?p=preview

@PaulL1
Copy link
Contributor

PaulL1 commented Oct 3, 2014

That is interesting. If you add 500, then 100, it has spare columns. But if you keep clicking on the 100 then the spare ones eventually go away.

The code relies on there always being a name, even though that's not documented. I meant to do something about that - in buildColumns we have this code:

    // Synchronize self.columns with self.options.columnDefs so that columns can also be removed.
    self.columns.forEach(function (column, index) {
      if (!self.getColDef(column.name)) {
        self.columns.splice(index, 1);
      }
    });

It looks up the columnDefs by column.name, and I think there's a couple of other places that assume there's a name. In 2.x you could just use field, but no more unfortunately.

More importantly, I think the problem may be that code is using splice inside a forEach. I have a niggling feeling that that is a bad idea, as it breaks the loop construct (the array is getting shorter as we're trying to iterate over it). I need to check. I have some changes outstanding on this area of code anyway (#1685) which tidies up a few other things. It still needs one patch once I work out how to do it, and I could fix this at the same time.

@PaulL1 PaulL1 self-assigned this Oct 3, 2014
@PaulL1 PaulL1 added this to the 3.0 milestone Oct 3, 2014
@6gsaifulislam
Copy link
Author

Thanks Paul - it is nice to have something which is "interesting" rather than it being caused by my lack of knowledge.
I am using ui grid as a means of editing collections in mongodb and the 205_row_editable features makes updating mongodb a lot easier as the collections now appear in spreadsheet mode rather than json.
If I could add to the wish list it would be an insert and delete row feature (not the router bit)
run the router bit and get the conformation that record has been deleted or inserted and at that point insert or delete from the grid. .

@PaulL1
Copy link
Contributor

PaulL1 commented Oct 3, 2014

In my application I do an insert, but I do it from outside the grid. I have a new button, and that pushes a new row to the end of the data. Then rowEdit calls the saveRow callback on it same as normal once the user is finished editing. In my callback I check whether it has an id so as to decide whether it's a save or update (using ngResource). I'm not sure that putting something extra into rowEdit would help - there's very little code in it. (And there's nowhere good to put a "new" button - it's better below the grid anyway).

Delete is perhaps more interesting, in my grid I just put a delete button on each row, and again handle from outside the grid. But it would perhaps benefit from staying in the grid until physically deleted from the database. I'll think on that.

PaulL1 added a commit to PaulL1/ng-grid that referenced this issue Oct 4, 2014
Tidies up much of the column behaviour.

Removed col.index everywhere it appears, except for in the cell content
templates.  For the cell content, replace with col.uid, which is now
uniquely generated when the column is created, and never changed again.

Should also fix angular-ui#1694, but need to push to be sure.
@PaulL1
Copy link
Contributor

PaulL1 commented Oct 4, 2014

OK, it's updated to the ui-grid.info site, and rechecking the plunkr it looks fixed.

@6gsaifulislam
Copy link
Author

Thanks Paul it works great!

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

Successfully merging a pull request may close this issue.

2 participants