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

🚨🚨🚨🚨 Resizing columns when columns are created as classes breaks 🚨🚨🚨🚨 #666

Open
leolorenzoluis opened this issue Apr 7, 2017 · 11 comments

Comments

@leolorenzoluis
Copy link

leolorenzoluis commented Apr 7, 2017

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

Current behavior
When resizing the column when columns are created as an instance of a class, then it breaks the table and the binding value still has the previous value. Here is the ff error:

TemplateRefTemplatesComponent.html:24 ERROR TypeError: v.context.column.breakIt is not a function
    at Object.View_TemplateRefTemplatesComponent_1.currVal_0 [as updateDirectives] (TemplateRefTemplatesComponent.html:24)
    at Object.debugUpdateDirectives [as updateDirectives] (core.es5.js:12613)
    at checkAndUpdateView (core.es5.js:12025)
    at callViewAction (core.es5.js:12340)
    at execEmbeddedViewsAction (core.es5.js:12312)
    at checkAndUpdateView (core.es5.js:12026)
    at callViewAction (core.es5.js:12340)
    at execEmbeddedViewsAction (core.es5.js:12312)
    at checkAndUpdateView (core.es5.js:12026)
    at callViewAction (core.es5.js:12340)
    at execComponentViewsAction (core.es5.js:12286)
    at checkAndUpdateView (core.es5.js:12031)
    at callViewAction (core.es5.js:12340)
    at execEmbeddedViewsAction (core.es5.js:12312)
    at checkAndUpdateView (core.es5.js:12026)

If I change it to like this then it doesn't break. But it loses the binding. It shows the previous value of the original object prior to resizing the column.

import { Component, TemplateRef, ViewChild } from '@angular/core'; export class TemplateRefTemplatesComponent {

@ViewChild('editTmpl') editTmpl: TemplateRef;
@ViewChild('hdrTpl') hdrTpl: TemplateRef;

rows = [];
columns = [];

constructor() {
this.fetch((data) => {
this.rows = data.splice(0, 5);
});
}

ngOnInit() {
let a = {};
a['cellTemplate'] = this.editTmpl;
a['headerTemplate'] = this.hdrTpl;
a['name'] = 'Gender';
a['giveme'] = () => {
console.log("blah");
}
a['breakIt'] = () => { if (b['query']) return true; return false };
let b = {};
b['cellTemplate'] = this.editTmpl;
b['headerTemplate'] = this.hdrTpl;
b['name'] = 'Name';
b['giveme'] = () => {
console.log("blah");
}
b['breakIt'] = () => { if (b['query']) return true; return false };
this.columns = [a,
b];
}

fetch(cb) {
const req = new XMLHttpRequest();
req.open('GET', assets/data/company.json);

req.onload = () => {
  cb(JSON.parse(req.response));
};

req.send();

}

blah() {
console.log(this.columns);
}

}

export class Test {
name: string;
cellTemplate: TemplateRef;
headerTemplate: TemplateRef;
query: string;

giveme() {
console.log(typeof (this));
console.log(this.query);
}

breakIt() {
if (this.query) return true; return false;
}
}

Expected behavior
It should not break.

Reproduction of the problem
http://run.plnkr.co/qY8q97gZkSeZ7Kvv/

Please tell us about your environment:
Windows 10, Node version 6.10.0

  • Table version: 0.7.1

  • Angular version: 4.0.x

  • Browser: [all | Chrome XX | Firefox XX | IE XX | Safari XX | Mobile Chrome XX | Android X.X Web Browser | iOS XX Safari | iOS XX UIWebView | iOS XX WKWebView ]
    All

  • Language: [Typescript 2.2.1]

@leolorenzoluis leolorenzoluis changed the title Resizing columns when columns are created as classes breaks 🚨🚨🚨🚨 Resizing columns when columns are created as classes breaks 🚨🚨🚨🚨 Apr 7, 2017
@amcdnl
Copy link
Contributor

amcdnl commented Apr 7, 2017

@arlowhite - I believe this is a regression of your PR today. Can you investigatE?

@arlowhite
Copy link
Contributor

@amcdnl ok, I'll take a look at it and try to submit a fix tomorrow.

@arlowhite
Copy link
Contributor

@leolorenzoluis I get a 404 from http://run.plnkr.co/qY8q97gZkSeZ7Kvv/

Is your Plunker example still online?

@leolorenzoluis
Copy link
Author

leolorenzoluis commented Apr 12, 2017

@arlowhite I don't know what happened to plunker, but if you expand the details I attached and use the demo app under Template-obj.component.ts found here https://github.com/swimlane/ngx-datatable/blob/master/demo/templates/template-obj.component.ts then you should be able to recreate it. Just replace it with the code that I attached. Let me know if you can't get it to work.

@arlowhite
Copy link
Contributor

@leolorenzoluis I don't get an error with your code.
I'm on master (8.0.0), Angular 4.0.2

I also tried instances of Test and it worked.

let a = new Test();
    a['cellTemplate'] = this.editTmpl;
    a['headerTemplate'] = this.hdrTpl;
    a['name'] = 'Gender';

Does 8.0.0 and the newest version of Angular work for you?

@arlowhite
Copy link
Contributor

@leolorenzoluis I don't see how breakIt() gets called. Did you also change the template HTML? could you paste that too?

@leolorenzoluis
Copy link
Author

@arlowhite Ah yes I changed the template in HTML, the breakIt function is attached inside a column header template. You can use *ngIf=breakIt()

@leolorenzoluis
Copy link
Author

leolorenzoluis commented Apr 21, 2017

@arlowhite Here's a plunker. http://plnkr.co/edit/J3qsdqFQUmWvVVzPwDR1?p=preview

Open the web debug tools and watch the console. Type something under column name's input control, then resize. Notice that it has the old value of the query.

Just in case plunkr disappears again. Here's the code:

//our root app component
import {Component, NgModule, TemplateRef, ViewChild, OnInit} from '@angular/core'
import {BrowserModule} from '@angular/platform-browser'
import { NgxDatatableModule } from '@swimlane/ngx-datatable';
import { FormsModule } from '@angular/forms';
import {
  TableOptions,
  TableColumn,
  ColumnMode
} from '@swimlane/ngx-datatable';

@Component({
  selector: 'my-app',
  template: `
    <div>
      <h3>
        TemplateRef via Column Property
         <small>
          <a href="https://github.com/swimlane/ngx-datatable/blob/master/demo/templates/template-obj.component.ts" target="_blank">
            Source
          </a>
        </small>
      </h3>
      OMG
      <ngx-datatable
        class="material"
        [rows]="rows"
        [columns]="columns"
        [columnMode]="'force'"
        [headerHeight]="50"
        [footerHeight]="50"
        [rowHeight]="'auto'">
      </ngx-datatable>
      <ng-template #hdrTpl let-column="column" >
      <input [(ngModel)]="column.query">
        <strong *ngIf="column.breakIt()">WHAT</strong>: {{column.name}} !!
      </ng-template>
      <ng-template #editTmpl let-row="row" let-value="value">
        <img
          *ngIf="value === 'male'"
          width="150"
          src="https://media.giphy.com/media/I8nepxWwlEuqI/giphy.gif"
        />
        <img
          *ngIf="value === 'female'"
          width="150"
          src="https://media.giphy.com/media/sxSVG3XHf7yww/giphy.gif"
        />
      </ng-template>
    </div>
  `
})
export class App implements OnInit {
  
@ViewChild('editTmpl') editTmpl: TemplateRef;
@ViewChild('hdrTpl') hdrTpl: TemplateRef;

  rows = [];

  constructor() {
    this.fetch((data) => {
      this.rows.push(...data);
    });
  }
  
  ngOnInit(){
    
    let a = {};
a['cellTemplate'] = this.editTmpl;
a['headerTemplate'] = this.hdrTpl;
a['name'] = 'Gender';
a['query'] = 'a';
a['giveme'] = () => {
console.log("blah");
}
a['breakIt'] = () => { if (a['query']) return true; return false };
let b = {};
b['cellTemplate'] = this.editTmpl;
b['headerTemplate'] = this.hdrTpl;
b['name'] = 'Name';
b['giveme'] = () => {
console.log("blah");
}
b['breakIt'] = () => { console.log(b['query']); if (b['query']) return true; return false };
this.columns = [a,
b];
  }

  fetch(cb) {
    const req = new XMLHttpRequest();
    req.open('GET', 'https://unpkg.com/@swimlane/[email protected]/assets/data/company.json');

    req.onload = () => {
      cb(JSON.parse(req.response));
    };

    req.send();
  }

}


@NgModule({
  imports: [ BrowserModule,FormsModule, NgxDatatableModule ],
  declarations: [ App ],
  bootstrap: [ App ]
})
export class AppModule {}

@arlowhite
Copy link
Contributor

Confirmed. Resize seems to do a clone of the column object.
The a or b reference is no longer the same as the column after resize.
http://plnkr.co/edit/Q4TqCPNqP452fchyT7op?p=info
a!==col and a.query!==col.query (after changing query)

You can work around this by passing in the column as an argument within the template and only using that parameter in your code:
column.breakIt(column)

This does seem like a bug that should be fixed.

@arlowhite
Copy link
Contributor

arlowhite commented Apr 21, 2017

@amcdnl the shallow-clone happens here: https://github.com/swimlane/ngx-datatable/blob/master/src/components/datatable.component.ts#L921

I'm not going to attempt a fix because I don't know enough about this code or the need for Object.assign here.
But I don't think this was caused by PR #494

@leolorenzoluis
Copy link
Author

leolorenzoluis commented Apr 22, 2017

Yeah that's what I thought. I just "re-attach" the binding the column and find it on my backing columns field and modify it from there. It's ugly but it works.

@amcdnl Just like arlow, I don't know enough about the code, but I can sense the functional programming principle here that it's trying to apply immutability, but why? As a consumer of the API, I would expect that if the internal columns are mutated as a new reference, then anything that is bound to the columns must be two-way binding.

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.

3 participants