Covalent: [ BUG ] datatable rowselect trigger rowclick even too

[hengkyz]

Bug Report

BUG (Not correct behavior)

What is the expected behavior?

i've tried rowclick event in datatable but got some bug
the rowselect method is not working properly,
so when you click the checkbox it will trigger the rowclick method instead of rowselect method
you can see the example in the documentation too

[manickam91]

is there a different event listeners for click and select , it doesn't work as expected.

[jotatoledo]

As far as I can see, clicking the checbox triggers both events. Most likely this is the expected behaviour by design, what in my opinion is correct because the checkbox element is still part of the row element.

[manickam91]

yes @jotatoledo , but if i have a navigation based on row click and deletion based on multi selected rows, this usecase fails. need to see what would be the right solution

[hengkyz]

@manickam91 there is a different when you click the row it will trigger the rowclickevent however for checkbox @jotatoledo you are right it will trigger both
but in my opinion in most case (especially enterprise application) it should be different,
for example:
if you click the row (not checkbox) you will redirect the page to the form which is detail data from the row that you clicked, whereas if you want to delete the data (multiple delete) you should click the row (checkbox) first than you click the button delete which is will delete the row (data) that you want to delete. However with current feature you cant do that, because if you click the checkbox you will redirect to the detail page because the checkbox will trigger the rowclick event (which is this row click event have redirect method to the detail page)

[hengkyz]

@manickam91 lol i write long explanation then you summarize all the need in simple paragraph ROFL
that @manickam91 statement really true, thats what i expect the correct behavior

[hengkyz]

in my opinion the checkbox should not trigger the rowclick event
@emoralesb05 @jeremysmartt

[jotatoledo]

I can understand the use case, lets wait and see what the dev team thinks about it

[jotatoledo]

A possible fix for this could be:

In covalent/src/platform/core/data-table/data-table.component.html

<tr td-data-table-row
        [tabIndex]="isSelectable ? 0 : -1"
        [class.mat-selected]="(isClickable || isSelectable) && isRowSelected(row)"
        *ngFor="let row of data; let rowIndex = index"
        (keyup)="isSelectable && _rowKeyup($event, row, rowIndex)"
        (keydown.space)="blockEvent($event)"
        (keydown.shift.space)="blockEvent($event)"
        (keydown.shift)="disableTextSelection()"
        (keyup.shift)="enableTextSelection()">
      <td td-data-table-cell class="mat-checkbox-cell" *ngIf="isSelectable">
        <md-pseudo-checkbox
          [state]="isRowSelected(row) ? 'checked' : 'unchecked'"
          (mousedown)="disableTextSelection()"
          (mouseup)="enableTextSelection()"
          (click)="select(row, $event, rowIndex)">
        </md-pseudo-checkbox>
      </td>
      <td td-data-table-cell
          (click)="handleRowClick(row, $event, rowIndex)"
          [numeric]="column.numeric"
          [hidden]="column.hidden"
          *ngFor="let column of columns">
        <span class="md-body-1" *ngIf="!getTemplateRef(column.name)">{{column.format ? column.format(getCellValue(column, row)) : getCellValue(column, row)}}</span>
        <ng-template
          *ngIf="getTemplateRef(column.name)"
          [ngTemplateOutlet]="getTemplateRef(column.name)"
          [ngOutletContext]="{ value: getCellValue(column, row), row: row, column: column.name }">
        </ng-template>
      </td>
    </tr>

Moving the above event/event handler from the tr element into the td td-data-table-cell (not the one for the checkbox element). Not sure if this is correct though.

[emoralesb05]

So as @jotatoledo mentioned, this was by design.

The issue was about having separate events, which now we have (rowClick) and (rowSelect).

There was never a specification of throwing one event or the other because when you actually do a selection, you technically are clicking on the row too, so the behavior is as designed.

Now we can sure discuss about if we should block the event if its a selection or something that lets the user know that, since i do understand the use case.

So lets try and be clear when opening issues, cause whats expected might not be whats been asked for.

[hengkyz]

@emoralesb05

but if i have a navigation based on row click and deletion based on multi selected rows, this usecase fails. need to see what would be the right solution

based on that scenario, we cant achieve it

[kyleledbetter]

Use multi select checkboxes and links on the names of the items using row templates

[hengkyz]

@kyleledbetter what does it mean?

[kyleledbetter]

Check the first example in the docs. It has a template for the row with button. Use that with routerLink and the selectable checkboxes and you'll have both features your need

[jotatoledo]

what @kyleledbetter suggests is the following if Im not wrong:

<td-data-table
  [data]="basicData"
  [columns]="columns">
  <ng-template tdDataTableTemplate="action" let-value="value" let-row="row" let-column="column">
    <div fxLayout="row">
      <a md-button routerLink=[row.id]>
         Navigate!
      </a>
    </div>
  </ng-template>
</td-data-table>

where action could be another column defined by ITdDataTableColumn in the columnsinput.

Still, most surely there are other use cases where somebody would like to only trigger rowSelectwhen the user clicks the checkbox element. Could it be possible to configurate this through a flag variable?

[emoralesb05]

Yeah, yesterday i was discussing this with @kyleledbetter and @jeremysmartt, and we might make it the default behavior.

Its just we have to find a way to block the angular click event when clicking on the checkbox. Ive never blocked an angular event, so im not sure if its simple or not.

[emoralesb05]

Created a small PR to address this. Not sure if its the correct way of doing it (couldnt think of any other way), but it was the less expensive way of doing it (time wise and code change wise).

[jeremysmartt]

Hey all I updated the PR so that there is a new attribute that we check for:
stopRowClick
So on that first cell with the checkbox we now have this:

<md-pseudo-checkbox
          [state]="isRowSelected(row) ? 'checked' : 'unchecked'"
          (mousedown)="disableTextSelection()"
          (mouseup)="enableTextSelection()"
          stopRowClick
          (click)="select(row, $event, rowIndex)">
</md-pseudo-checkbox>

Notice the stopRowClick attribute that is on there. This lets us know not to send the rowClick Event when you click on that. Also, the nice thing about this is it can be used on any cells not just this first one with the checkbox. So if you add any other buttons or anything in any other cells and don't want the the rowClick to happen then just add this attribute.