angular / components

Component infrastructure and Material Design components for Angular
https://material.angular.io
MIT License
24.32k stars 6.73k forks source link

Selection model for Mat Table 2 has defect #11613

Closed tatsujb closed 6 years ago

tatsujb commented 6 years ago

Bug, feature request, or proposal:

It is impossible to feed this.selection.select and this.selection.toggle a carbon copy of row but row is accepted.

It goes further, lines selected thereafter with the "normal" row, will also result in correct selection even though the whole process should crash and fail now that selection has to iterate through "faulty" items as well.

What is the expected behavior?

that a passed object that that is identical to the object that row is would work too.

What is the current behavior?

html :

...
       </div>
          </mat-cell>
        </ng-container>

        <mat-header-row *matHeaderRowDef="pinnedColumnsWSelect"></mat-header-row>
        <mat-row *matRowDef="let row; columns: pinnedColumnsWSelect;"
                 class="noselect"
                 [ngClass]="{ 'selected': selection.isSelected(row)}"
                 (click)="addToSelection(row, $event, false)"></mat-row>
      </mat-table>
...

(false is for, did the click originate from a checkbox element or from just clicking on the row?)

ts :

  addToSelection(row, event, checkbox){
    if(event.shiftKey && this.previous !== -1 && this.previous !== row.numberOfRow) {
      if(this.previous > row.numberOfRow){
        for(let previous = this.previous - 1; previous >= row.numberOfRow; previous-- ){
          this.selection.toggle(this.originalDataSet.filter(x => x['numberOfRow'] === previous)[0] as object[]);
        }
      }else{
        for(let previous = this.previous + 1; previous <= row.numberOfRow; previous++ ){
          this.selection.toggle(this.originalDataSet.filter(x => x['numberOfRow'] === previous)[0] as object[]);
        }
      }
    } else if(event.ctrlKey || checkbox) {
      this.selection.toggle(row);
    } else {
      if(this.originalDataSet){
        if(this.selection.selected.length === 1 && this.selection.selected[0] === row) {
        this.selection.clear();
      } else {
          this.selection.clear();
          this.selection.select(row);
        }
      }
    }
    this.previous = row.numberOfRow;
  }

as you can see above I first check if the shift key was held down if it was I apply my selection which is currently causing issues, if shift isn't being held but ctrl is I add to selection (this works), and lastly I'm in a case where neither keys are being held down, I simply clear the selection and set the new item as selected.

simple enough right?

As you may conclude from reading my above code, what I'm trying to do in the "shift held down" part is obtain the corresponding row for each line to be selected and pass that to selection's toggle function.

having previously console.logged row and noticed that row was indeed the entire current row of the table I deduced that I could emulate the correct objects being passed to the method "toggle" between points A and B.

My material table accepts an array of objects as it's dataset. Each object corresponds to a row, each object's keys corresponds to the table's headers. so far so good.

Pulling the right row from the array by myself (with a filter where I match the numberOfRow, my unique identifier which happens to count rows (0 , 1, 2, 3, ect...)) should give me the same thing.

console logging the two give me the same thing :

const y = this.originalDataSet.filter(x => x['numberOfRow'] === previous)[0];
console.log('filtered item ', y, ' row ', row, ' equal ', y === row, y == row );

however the nightmare begins at the two declaring being neither equal partially or entirely (with type).

now for === fair enough but for ==, why??

how this happens I have no idea filtered item {numberOfRow: 2, nCommande: "4500131111", nLigne: "00010", nEcheance: "0001", id: {…}, …} row {numberOfRow: 2, nCommande: "4500131111", nLigne: "00010", nEcheance: "0001", id: {…}, …} equal false false image

this is something I legitimately have never ever seen before in javascript. the two objects are IDENTICAL I checked manually 15 times now by opening all the nodes. yet == fails.

but hold that thought, mesmerizing though it is, I have something even more mesmerizing for you.

What are the steps to reproduce?

let's console.log our selection (console.log(this.selection.selected);)

  1. then click the first Itemof our mat-table
  2. shift-click the fourth
  3. ctrl click the fith

what would you expect happens ?

that none end up selected because selection is in an incorrect format? I'd love that that only the first end up selected because selection's array is incorrect beyond that point? this would also make sense that all 5 items be correctly selected? one can dream

well no :
image

ok let's look at the log :

(5) [{…}, {…}, {…}, {…}, {…}]
0 : {numberOfRow: 0, nCommande: "2284595", nLigne: "1", nEcheance: "0", id: {…}, …}
1 : {numberOfRow: 1, nCommande: "2284595", nLigne: "2", nEcheance: "0", id: {…}, …}
2 : {numberOfRow: 2, nCommande: "4500131111", nLigne: "00010", nEcheance: "0001", id: {…}, …}
3 : {numberOfRow: 3, nCommande: "4500131111", nLigne: "00020", nEcheance: "0001", id: {…}, …}
4 : {numberOfRow: 4, nCommande: "4500634818", nLigne: "00010", nEcheance: "0001", id: {…}, …}
length : 5
__proto__:Array(0)

I'm confused.

you correctly selected array item 0 and 4 using this array but for everything in between, no-go, even though 4's selection (item 5) came last.

how?

this pattern of "distinguishing" between "faulty" and "correct" where the human eye can't, continues as you stack on shift selections and ctrl selections ad infinitum. If your this.selection.selected is 100 000 items long it will still not have selected in the actual visual representation all those items added in it's array with shift and have correctly selected all those added with ctrl.

let's mix it up because that's how we get something that finally diverges from the beaten path.

let's try deselecting with shift

If I do one more shit click intending to add three more items and one more control click for the item below that then shift click back UP to the second item : image

nothing diverging from our current mess, at least visually :

(9) [{…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}]
0 : {numberOfRow: 0, nCommande: "2284595", nLigne: "1", nEcheance: "0", id: {…}, …}
1 : {numberOfRow: 1, nCommande: "2284595", nLigne: "2", nEcheance: "0", id: {…}, …}
2 : {numberOfRow: 2, nCommande: "4500131111", nLigne: "00010", nEcheance: "0001", id: {…}, …}
3 : {numberOfRow: 3, nCommande: "4500131111", nLigne: "00020", nEcheance: "0001", id: {…}, …}
4 : {numberOfRow: 4, nCommande: "4500634818", nLigne: "00010", nEcheance: "0001", id: {…}, …}
5 : {numberOfRow: 5, nCommande: "4500634818", nLigne: "00020", nEcheance: "0001", id: {…}, …}
6 : {numberOfRow: 6, nCommande: "4500634818", nLigne: "00030", nEcheance: "0001", id: {…}, …}
7 : {numberOfRow: 7, nCommande: "4500634818", nLigne: "00040", nEcheance: "0001", id: {…}, …}
8 : {numberOfRow: 8, nCommande: "4500634818", nLigne: "00050", nEcheance: "0001", id: {…}, …}
length : 9

(4) [{…}, {…}, {…}, {…}]
0 : {numberOfRow: 0, nCommande: "2284595", nLigne: "1", nEcheance: "0", id: {…}, …}
1 : {numberOfRow: 4, nCommande: "4500634818", nLigne: "00010", nEcheance: "0001", id: {…}, …}
2 : {numberOfRow: 8, nCommande: "4500634818", nLigne: "00050", nEcheance: "0001", id: {…}, …}
3 : {numberOfRow: 4, nCommande: "4500634818", nLigne: "00010", nEcheance: "0001", id: {…}, …}
length : 4

well this is unexpected.

why did it keep item 4 instead of deselecting it like the rest and on top of that ADDED IT AGAIN?

this makes me think that there is another array that's being used for comparisons I'm not aware of.

Furthermore

onChange results are exactly as expected :

  ngOnInit() {
    this.selection.onChange.subscribe(x=> {
      console.log(x);
    });
  }

for shift-click from first item to sixth and back again :

{source: SelectionModel, added: Array(1), removed: Array(0)}
{source: SelectionModel, added: Array(1), removed: Array(0)}
{source: SelectionModel, added: Array(1), removed: Array(0)}
{source: SelectionModel, added: Array(1), removed: Array(0)}
{source: SelectionModel, added: Array(1), removed: Array(0)}
{source: SelectionModel, added: Array(1), removed: Array(0)}
{source: SelectionModel, added: Array(0), removed: Array(1)}
{source: SelectionModel, added: Array(0), removed: Array(1)}
{source: SelectionModel, added: Array(0), removed: Array(1)}
{source: SelectionModel, added: Array(0), removed: Array(1)}
{source: SelectionModel, added: Array(1), removed: Array(0)}

I check the contents they indicate being exactly as they should be (the right item numbers in the right order).

yet the visual does not follow suit.

another experiment :

I mentioned in the intro that a carbon copy of the object would be refused : this is true.

If I do this to our so far functional ctrl code it ceases to function (this is the imported underscorejs library btw, it is a shallow clone, but it does not omit underlings, it uses their memory reference) :

} else if(event.ctrlKey || checkbox) {
      const bb = _.clone(row);
      this.selection.toggle(bb);
}

same thing happens with this approach (this is a deep clone) :

} else if(event.ctrlKey || checkbox) {
      const bb = jQuery.extend(true, {}, row);
      this.selection.toggle(bb);
}

the rows are no longer visually selected with ctrl-click yet the console logged selection array and all other aspects of selection continue to be faultless.

What is the use-case or motivation for changing an existing behavior?

for example adding shift-select support. but I don't see how everyone else with every other scenario wouldn't benefit from this bug being fixed. selection model would be a nice touch on mat table. right now though it's not an option for me.

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

windows pro 10 64bit chrome

{
  "name": "web.ui",
  "version": "0.0.0",
  "license": "MIT",
  "scripts": {
    "ng": "ng",
    "start": "ng serve --aot",
    "build": "ng b --prod",
    "test": "ng test",
    "lint": "ng lint"
  },
  "private": true,
  "dependencies": {
    "@angular/animations": "^6.0.3",
    "@angular/cdk": "^6.1.0",
    "@angular/common": "^6.0.3",
    "@angular/compiler": "^6.0.3",
    "@angular/core": "^6.0.3",
    "@angular/forms": "^6.0.3",
    "@angular/http": "^6.0.3",
    "@angular/material": "^6.1.0",
    "@angular/platform-browser": "^6.0.3",
    "@angular/platform-browser-dynamic": "^6.0.3",
    "@angular/router": "^6.0.3",
    "@types/underscore": "^1.8.7",
    "angular-font-awesome": "^3.1.2",
    "bootstrap": "^4.0.0",
    "classlist.js": "^1.1.20150312",
    "core-js": "^2.5.3",
    "file-saver": "^1.3.8",
    "font-awesome": "^4.7.0",
    "jquery": "^3.3.1",
    "lodash": "^4.17.5",
    "ng2-ion-range-slider": "^2.0.0",
    "ngx-bootstrap": "^3.0.0",
    "ngx-dropzone-wrapper": "^6.1.0",
    "rxjs": "^6.2.0",
    "rxjs-compat": "^6.0.0-rc.0",
    "typescript": "2.7.2",
    "underscore": "^1.8.3",
    "web-animations-js": "^2.3.1",
    "zone.js": "^0.8.20"
  },
  "devDependencies": {
    "@angular-devkit/build-angular": "~0.6.5",
    "@angular/cli": "^6.0.5",
    "@angular/compiler-cli": "^6.0.3",
    "@angular/language-service": "^6.0.3",
    "@types/jasmine": "^2.8.6",
    "@types/jasminewd2": "~2.0.3",
    "@types/node": "~10.1.3",
    "codelyzer": "^4.2.1",
    "postcss-modules": "^1.1.0",
    "protractor": "~5.3.0",
    "ts-node": "~6.0.5",
    "tslint": "~5.10.0"
  }
}
tatsujb commented 6 years ago

Ok I figured it out and this will seem to some of you like it was an unknown in the problem I gave and so you couldn't have guessed it because I didn't give you my full components and to the rest of you it will seem obvious, because you know mat table well enough and you could tell from the naming of my variable something was amiss,

I have a set of four local vars for the four steps of the treatment of my object between when it arrives from API and when it shows up visually in the mat-table.

  // [ 1 ] this first gets the api object and will serve as a store
  originalDataSet:Array<object>;

  // [ 2 ] rendered after filters
  filteredDataSet:Array<object>;

  // [ 3 ] sclice step (needs to be second to last due to dual slider for size)
  dataSlice = [];

  // [ 4 ] final. what actually is queried by Mat Table
  @ViewChild(MatSort) sort: MatSort;
  dataSource: MatTableDataSource<any>;

the rows (objects) are indeed the same between 1 and 4 but alot of them have gone missing since.

this does not answer why javascript thinks two copies of an object aren't the same but it does answer why the object I was feeding my toggle() methods was being considered not the same as row

now with this.dataSource.filteredData I'm literally iterating over the same array as my mat table is.

and this does not cause defects with switching pages and shift-clicking for example.

(apart from not selecting the items from page one it can no longer iterate over since they aren't in the present list, which is what I wanted to avoid at the start but I'll take that bug over this one)

If someone has an answer as to why js can't see a copy of an object as equal to itself I'd love to have an answer.

If someone can see how this object could fail to meet mat table's selection model's criteria and how I could have fooled it, I would also love to know.

angular-automatic-lock-bot[bot] commented 5 years ago

This issue has been automatically locked due to inactivity. Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.