feat(data-table): onclick event for datatable rows, select event only on checkboxes, multi shift click and basic a11y (closes #468) (closes #592)

[jeremysmartt]

Description

feat(data-table): (rowClick) event for datatable rows enabled by new [clickable] input.
feat(data-table): select event will be trigger only when clicking on checkbox.
feat(data-table): shift-click for multiple row selection/deselection
feat(data-table): improved keyboard a11y for row selection (space, enter, up, down)

What's included?

• modified: src/platform/core/data-table/data-table.component.html
• modified: src/platform/core/data-table/data-table.component.spec.ts
• modified: src/platform/core/data-table/data-table.component.ts

Test Steps

• checkout branch
• ng serve
• go to http://localhost:4200/#/components/data-table
• On table "Data Table with components" see that selecting a row still works
• Run unit tests and see that event is emitted and caught

General Tests for Every PR

• ng serve --aot still works.
• npm run lint passes.
• npm test passes and code coverage is not lower.
• npm run build still works.

[kyleledbetter]

Is it possible to add another option?

I'd like to have a selectable row w/ the checkbox, but only if you click the checkbox element and not the entire row. This is for when you have other clickable buttons and elements in columns in the row.

[hengkyz]

hi @jeremysmartt is it ready to test?
can i use it yet?
if i want to patch it on my covalent module how to do it?
i try to change the code but did not work?
any idea?

[emoralesb05]

The code is not released yet. Once is merged you, can install it from the nightly repo if you want to start testing it before its released.

npm install https://github.com/Teradata/covalent-nightly.git

edit: wont be in the nightly build until its merged.

[jeremysmartt]

@hengkyz Please feel free to test it out in our nightly build and give feedback on whether it is what you are expecting or not.

[hengkyz]

ok @jeremysmartt i will check on it,
will give you feedback after that

[hengkyz]

i did install from nightly build but i did not see the code you change in there,
is it merged already?
npm install --save https://github.com/Teradata/covalent-nightly.git

[kyleledbetter]

Nightly is built off master. This hasn't been merged yet

[hengkyz]

@kyleledbetter ok thank you

[kyleledbetter]

any chance we can sneak in some keyboard support for tabbing through rows, selecting one/multiple, etc? :) (MVP of course)

[emoralesb05]

Unit tests are failing

Also needs unit tests for the shift click enhancement

[jeremysmartt]

unit test added

[emoralesb05]

Dont know if the a11y of the shift click is correct, it starts behaving weird when i keep clicking around while holding shift.. while i would expect that the first place i clicked to be the reference as long as i keep holding it.

e.g.

Although since it this case for the data table is select/unselect.. im still wondering how it should behave.

[emoralesb05]

We should avoid using window and check for document since it might affect server side rendering later on.

Also i think its better to bind the event to the md-pseudo-checkbox and be sure it comes from it, instead of comparing the tagName

e.g.

  /**
   * Overrides the onselectstart method of the document so other text on the page
   * doesn't get selected when doing shift selections.
   */
  disableOnSelectStart(): void {
    if (document) {
      document.onselectstart = function(): boolean {
        return false;
      };
    }
  }

  /**
   * Resets the original onselectstart method.
   */
  enableOnSelectStart(): void {
    if (document) {
      document.onselectstart = undefined;
    }
  }

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

Which we can probably create a directive later on for this if we want to reuse the functionality.

[emoralesb05]

Also is better to import document from the constructor as optional and then use it

@Optional() @Inject(DOCUMENT) private _document: any

[emoralesb05]

🍷

[emoralesb05]

Added some commits for basic keyboard a11y.

[emoralesb05]

Also simplified the code for shift selection

chore(): simplify shift selection to use index

before it was finding the selected rows, but as a user i would expect to always toggle from first index to last index

also its better to simplify it for performance issues, since with lots of rows this couldve take a toll

[hengkyz]

wow good news all check have passed,
does it mean we just need to wait the PR to be merge?
@kyleledbetter @jeremysmartt @emoralesb05

[emoralesb05]

@hengkyz yep, we are doing the last polish steps since its more than just the clickRow event.

Once we merge it, we will release beta.4 and it will be good to go

[hengkyz]

i've tried this 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

[hengkyz]

hi @emoralesb05 @jeremysmartt any idea about my report above?

[emoralesb05]

Bug reports should be created as a separate issue, almost nobody looks at merged PR's. 😄

Please open an issue with the bug or behavior you were expecting and be as detailed as possible.

[hengkyz]

ok sorry don't know about that