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

Route change event from table row click disrupts Angular change detection #1232

Open hoffination opened 6 years ago

hoffination commented 6 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 I have a ngx-datatable that I have setup with each row to act like a button to route to another view. On click, a route change is triggered and the view is replace by another page which should destroy the page the table is on. This seems to work great on Chrome but I'm having issues on Edge.

When Edge makes the change there is a bug where the following view isn't rendered unless I trigger change detection manually with ChangeDetectorRef.detectChanges(). The routed view seems to only render the template with little-to-no bootstrapping of the component. The original page is still visible below the new view.

When I debug the problem it looks like the constructor is called but not the ngOnInit method. My current work-around is to run a setTimeout in the constructor and run change detection.

Another layer of strangeness is I'm only getting this bug on clicks from the second or later pages of content in the datatable. Clicks in the first rendered page seem to be fine and bootstrap the new route correctly. I'm also not getting any errors in the Edge console.

Expected behavior

Upon clicking on a row in the datatable the click handler would change the route and the following view will bootstrap correctly. The table will allow change detection to trigger regularly and process the route change appropriately.

Reproduction of the problem

I'll try to reproduce in a plnkr but here's an example of my current table use:

<ngx-datatable
    #datatable
    class="material striped"
    [headerHeight]="50"
    [footerHeight]="50"
    [rows]="outputRows$ | async"
    [selected]="selected"
    [selectionType]="'checkbox'"
    (select)='onSelect($event)'
    columnMode="flex"
    [selectAllRowsOnPage]="false"
    [rowHeight]="50"
    [scrollbarV]="true"
    (activate)="handleEvent($event)"
  >
    <ng-content></ng-content>
  </ngx-datatable>

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

Seems to be a serious bug on Edge.

Please tell us about your environment:

Basic Angular CLI built project served up on OSX and run on Browserstack's Edge 16 browser. Also reproduced on Windows 10 running Edge 16.

molcik commented 6 years ago

I have the same issue on chrome. I noticed that this happens when the table is scrolling and I click the row.

hoffination commented 6 years ago

@molcik I also started seeing this problem occur on Chrome for my app as well. It looks like I found a fix for my app. Can you try out my fix and see if it resolves the issue on your end?

555ea commented 6 years ago

@hoffination Sorry for offtopic, could you please provide your code of using <ng-content></ng-content> inside of <ngx-datatable></ngx-datatable>

? Can't get it to work, _internalColumns are undefined and can't find any solution on the net. Are you calling anything on #datatable ? datatable.recalculate() didn't help me

The only workaround I've found so far is to get @ContentChildren inside of wrapper component and pass them to datatable like

@ViewChild(DatatableComponent) datatable: DatatableComponent; 
@ContentChildren(DataTableColumnDirective) val: QueryList<DataTableColumnDirective>;

  ngOnInit() {
    setTimeout(() => {
      this.datatable.columnTemplates = this.val;
    });
  }
hoffination commented 6 years ago

@555ea I can give you a couple snippets that might help out:

First here's the definition for the ContentChildren:

@ContentChildren(ColumnComponent) set columns(val: QueryList<DataTableColumnDirective>) {
    this._columns = val;
  }

Next, we assign the column templates in an ngAfterContentInit:

ngAfterContentInit() {
    this.datatable.columnTemplates = this.columns;
}

Hope this helps. We ended up going with AgGrid for our implementation because it delivered more of the features we were hoping for with less zone-related complication.

Koslun commented 6 years ago

Also noticing this behavior in Chrome, version 64 to be specific.

I am still not sure when this bug became relevant for Chrome but only started hearing reports from our users regarding this issue the last couple of days, where we haven't updated any dependencies since January 16th (locking dependencies with yarn). So think it might not be present in Chrome version 63 but present in 64 and onwards.

Dependencies:

Angular version "5.1.3"
zone.js version "0.8.18"
TypeScript version "2.5.3"
Angular-cli version "1.6.3"
@swimlane/ngx-datatable version "11.1.7"
Koslun commented 6 years ago

None of the mentioned quick-fixes worked for me. Though I could get a value for DatatableComponent I could never get a value a for columns. Could not either find a place where calling ChangeDetectorRef.detectChanges() helped. After navigation the current component is destroyed, while the component that is navigated to lacks bootstrapping and does seemingly cannot call it.

@hoffination's fork linked above did however work. For me meaning:

"@swimlane/ngx-datatable": "https://github.com/hoffination/ngx-datatable",

Now locked in yarn.lock to the commit linked above.

@hoffination Given that you've moved on to another table solution, do you have any intention or interest in creating a PR?

hoffination commented 6 years ago

@Koslun Glad to hear that the fork was able to resolve your issue. I put in a PR in late January but the repository owner didn't like the potential for slowdown that came with my solution. He suggested using ngZone within your callback handlers which worked well for me instead of my fork. Here's a link to the PR:

https://github.com/swimlane/ngx-datatable/pull/1238