swimlane / ngx-datatable

✨ A feature-rich yet lightweight data-table crafted for Angular
http://swimlane.github.io/ngx-datatable/
MIT License
4.63k stars 1.68k forks source link

modifying [selected] input after using header check broken. #818

Open arlowhite opened 7 years ago

arlowhite commented 7 years ago

I'm submitting a ... (check one with "x")

[x] bug report => search github for a similar issue or PR before submitting
[ ] feature request
[ ] support request => Please do not submit support request here, post on Stackoverflow or Gitter

Current behavior

Certain table UI actions cause ngxDatatable.selected to be a different array from the original [selected] input. i.e. ngxDatatable.selected !== mySelectedRowsObject This breaks selection by mutating mySelectedRowsObject (push/splice)

Expected behavior

[selected] should always correspond with what is checked in the table.

Reproduction of the problem

http://swimlane.github.io/ngx-datatable/#chkbox-selection

  1. click header check to select all
  2. click header check again to de-select all
  3. click Add in toolbar, which pushes object into [selected] array

Table does not show checks for selected rows even though listed under Selections.

What is the motivation / use case for changing the behavior?

fix selection bug.

Please tell us about your environment:

Official ngx-datatable demo.

arlowhite commented 7 years ago

FYI, one work around for this bug is just to not use [selected] and modify table.selected directly.

  @ViewChild(DatatableComponent)
  table: DatatableComponent;
thejewdude commented 7 years ago

This issue occurs only in the demo because it uses array.push to change the select elements instead of assigning a new array. By default Angular will detect changes when the object reference changes .Which means if we mutate/change the array it won't detect the changes.

The demo can be fixed by changing a few things:

onSelect({ selected }) {
  console.log('Select Event', selected, this.selected);

  // this.selected.splice(0, this.selected.length);
  // this.selected.push(...selected);
  this.selected = selected;
}

add() {
  this.selected = [...this.selected, this.rows[1], this.rows[3]];
  // this.selected.push(this.rows[1], this.rows[3]);
}
arlowhite commented 7 years ago

@yehudag I'm not sure this is a ChangeDetection issue. Mutating the array works at first, but certain selection actions in the table create a new array object, which stops mutation from working. Though in the future, if more ngx-datatable components are changed to OnPush mutation would probably stop working. In any case, if developers are expected to re-assign the selected object from onSelect to their selected input, I think this should be documented in the API reference. https://swimlane.gitbooks.io/ngx-datatable/content/api/table/inputs.html

Adondriel commented 7 years ago

I have a similar issue to this... I am using my grid as close to an "excel-like" grid as possible. I have it so that, pressing enter, while focused on an input, will select and enable editing on the next row below it, because I only want 1 row to be selected at a time (for now at least, reqs may change in the future). The issue i'm getting is that, after pressing enter, it does it's thing, everything works fine, but when you go to select another row, it spits out this error:

ERROR Error: ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: 'datatable-body-cell pr-1 sort-active active'. Current value: 'datatable-body-cell pr-1 sort-active'.
    at viewDebugError (core.es5.js:8711)
    at expressionChangedAfterItHasBeenCheckedError (core.es5.js:8689)
    at checkBindingNoChanges (core.es5.js:8853)
    at checkNoChangesNodeInline (core.es5.js:12706)
    at checkNoChangesNode (core.es5.js:12680)
    at debugCheckNoChangesNode (core.es5.js:13455)
    at debugCheckRenderNodeFn (core.es5.js:13395)
    at Object.eval [as updateRenderer] (DataTableBodyRowComponent.html:6)
    at Object.debugUpdateRenderer [as updateRenderer] (core.es5.js:13377)
    at checkNoChangesView (core.es5.js:12502)

Luckily, it seems to just be some kind of warning error, because it doesn't stop anything from happening, I just don't like seeing random errors that I can't find a fix for. None of the above solutions worked for me, because I have to clear the list, currently, I do this by setting: this.selectedRows = []; Is there another way I should be doing this? I tried the =[...] thing, but it just created MORE errors... maybe it's because my onSelect() method isn't modifying the this.selectedRows array?

Arti3DPlayer commented 3 years ago

+1