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

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

Merged
merged 31 commits into from
May 16, 2017
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
027d50e
feat: Datatable: onclick event for datatable rows
May 8, 2017
a4e4460
changing event name to ITdDataTableRowClickEvent
May 8, 2017
cf2d51a
Merge branch 'develop' into feature/datatable-row-onclick
jeremysmartt May 8, 2017
1828a4a
merging develop into branch
May 8, 2017
eec687e
Merge branch 'develop' into feature/datatable-row-onclick
jeremysmartt May 8, 2017
632453d
Merge branch 'develop' into feature/datatable-row-onclick
May 12, 2017
01c777b
update to have selectable row w/ the checkbox, but only if you click …
May 12, 2017
c28d148
adding ability to shift multi select rows in datatable
May 12, 2017
94e81e9
updating demo text to say ability to shift click and adding a unit te…
May 13, 2017
35b1eb3
Merge branch 'develop' into feature/datatable-row-onclick
emoralesb05 May 15, 2017
3a09de4
Merge branch 'develop' into feature/datatable-row-onclick
emoralesb05 May 15, 2017
8f9a666
chore(): expose ITdDataTableRowClickEvent in the API
emoralesb05 May 15, 2017
365edfd
feat(): use DOCUMENT if its there, else dont use it
emoralesb05 May 15, 2017
5409ae9
chore(): add (rowClick) in data-table docs
emoralesb05 May 15, 2017
1ebdd62
chore(data-table): remove checked arg in select method
emoralesb05 May 15, 2017
cb189ee
Merge branch 'develop' into feature/datatable-row-onclick
kyleledbetter May 15, 2017
ec038aa
chore(data-table): test the click event as output
emoralesb05 May 15, 2017
1281a51
feat(data-table): adding keyboard a11y for selections
emoralesb05 May 15, 2017
dd389f1
feat(data-table): add more keyboard a11y
emoralesb05 May 15, 2017
f453f6e
Merge branch 'develop' into feature/datatable-row-onclick
emoralesb05 May 15, 2017
1117342
chore(): simplify shift selection to use index
emoralesb05 May 15, 2017
565ec7f
Fixing case where selecting a row, then unselecting the row and then …
May 15, 2017
15a1a26
Ability for Holding Shift Key and moving up and down the datatable wi…
May 15, 2017
b9a0a45
clickable input to datatable allows for rowClick event to be fired. …
May 16, 2017
cbd2116
chore(): use dialog instead of alert
emoralesb05 May 16, 2017
980cbbf
fix(): be able to tab when its `selectable`
emoralesb05 May 16, 2017
b1bbc98
fix for when multiple is false and only allowing 1 selection and fix …
May 16, 2017
7cfa95b
fix unit tests
May 16, 2017
411af75
chore(): fixed single selection
emoralesb05 May 16, 2017
9bd4f9c
Merge branch 'feature/datatable-row-onclick' of https://github.com/Te…
emoralesb05 May 16, 2017
5fc7731
Merge branch 'develop' into feature/datatable-row-onclick
emoralesb05 May 16, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ <h3>No results to display.</h3>
Selectable rows
</md-slide-toggle>
<md-slide-toggle class="push-left" color="accent" [(ngModel)]="multiple" [disabled]="!selectable">
Select multiple rows
Select multiple rows (shift + click for range selection)
</md-slide-toggle>
</div>
<md-divider></md-divider>
Expand Down
5 changes: 3 additions & 2 deletions src/platform/core/data-table/data-table.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@
<tr td-data-table-row
[class.mat-selected]="isSelectable && isRowSelected(row)"
*ngFor="let row of data"
(click)="isSelectable && select(row, !isRowSelected(row), $event)">
(click)="clickRow(row)">
<td td-data-table-cell class="mat-checkbox-cell" *ngIf="isSelectable">
<md-pseudo-checkbox
[state]="isRowSelected(row) ? 'checked' : 'unchecked'">
[state]="isRowSelected(row) ? 'checked' : 'unchecked'"
(click)="select(row, !isRowSelected(row), $event)">
</md-pseudo-checkbox>
</td>
<td td-data-table-cell
Expand Down
77 changes: 70 additions & 7 deletions src/platform/core/data-table/data-table.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
import 'hammerjs';
import { Component } from '@angular/core';
import { By } from '@angular/platform-browser';
import { MdPseudoCheckbox } from '@angular/material';
import { TdDataTableColumnComponent } from './data-table-column/data-table-column.component';
import { TdDataTableRowComponent } from './data-table-row/data-table-row.component';
import { TdDataTableComponent, ITdDataTableColumn } from './data-table.component';
Expand Down Expand Up @@ -169,7 +170,7 @@ describe('Component: DataTable', () => {
expect(dataTableComponent.indeterminate).toBeFalsy();
expect(dataTableComponent.allSelected).toBeFalsy();
// select a row with a click event
fixture.debugElement.queryAll(By.directive(TdDataTableRowComponent))[2].triggerEventHandler('click', new Event('click'));
fixture.debugElement.queryAll(By.directive(MdPseudoCheckbox))[2].triggerEventHandler('click', new Event('click'));
fixture.detectChanges();
fixture.whenStable().then(() => {
// check to see if its in indeterminate state
Expand Down Expand Up @@ -225,23 +226,23 @@ describe('Component: DataTable', () => {
expect(dataTableComponent.indeterminate).toBeFalsy();
expect(dataTableComponent.allSelected).toBeFalsy();
// select a row with a click event
fixture.debugElement.queryAll(By.directive(TdDataTableRowComponent))[2].triggerEventHandler('click', new Event('click'));
fixture.debugElement.queryAll(By.directive(MdPseudoCheckbox))[2].triggerEventHandler('click', new Event('click'));
fixture.detectChanges();
fixture.whenStable().then(() => {
// check to see if its in indeterminate state
expect(dataTableComponent.indeterminate).toBeTruthy();
expect(dataTableComponent.allSelected).toBeFalsy();
// select the rest of the rows
fixture.debugElement.queryAll(By.directive(TdDataTableRowComponent))[1].triggerEventHandler('click', new Event('click'));
fixture.debugElement.queryAll(By.directive(TdDataTableRowComponent))[3].triggerEventHandler('click', new Event('click'));
fixture.debugElement.queryAll(By.directive(TdDataTableRowComponent))[4].triggerEventHandler('click', new Event('click'));
fixture.debugElement.queryAll(By.directive(MdPseudoCheckbox))[0].triggerEventHandler('click', new Event('click'));
fixture.debugElement.queryAll(By.directive(MdPseudoCheckbox))[1].triggerEventHandler('click', new Event('click'));
fixture.debugElement.queryAll(By.directive(MdPseudoCheckbox))[3].triggerEventHandler('click', new Event('click'));
fixture.detectChanges();
fixture.whenStable().then(() => {
// check to see if its in indeterminate state and allSelected
expect(dataTableComponent.indeterminate).toBeTruthy();
expect(dataTableComponent.allSelected).toBeTruthy();
// unselect one of the rows
fixture.debugElement.queryAll(By.directive(TdDataTableRowComponent))[2].triggerEventHandler('click', new Event('click'));
fixture.debugElement.queryAll(By.directive(MdPseudoCheckbox))[2].triggerEventHandler('click', new Event('click'));
fixture.detectChanges();
fixture.whenStable().then(() => {
// check to see if its in indeterminate state and not allSelected
Expand All @@ -255,8 +256,70 @@ describe('Component: DataTable', () => {
})();
});

});
it('should shift click and select a range of rows',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍷

(done: DoneFn) => { inject([], () => {
let fixture: ComponentFixture<any> = TestBed.createComponent(TdDataTableSelectableTestComponent);
let element: DebugElement = fixture.debugElement;
let component: TdDataTableSelectableTestComponent = fixture.debugElement.componentInstance;

component.selectable = true;
component.multiple = true;
component.columns = [
{ name: 'sku', label: 'SKU #' },
{ name: 'item', label: 'Item name' },
{ name: 'price', label: 'Price (US$)', numeric: true },
];

component.data = [{ sku: '1452-2', item: 'Pork Chops', price: 32.11 },
{ sku: '1421-0', item: 'Prime Rib', price: 41.15 },
{ sku: '1452-1', item: 'Sirlone', price: 22.11 },
{ sku: '1421-3', item: 'T-Bone', price: 51.15 }];

fixture.detectChanges();
fixture.whenStable().then(() => {
let dataTableComponent: TdDataTableComponent = fixture.debugElement.query(By.directive(TdDataTableComponent)).componentInstance;
// check how many rows (without counting the columns) were rendered
expect(fixture.debugElement.queryAll(By.directive(TdDataTableRowComponent)).length - 1).toBe(4);
// check to see checkboxes states
expect(dataTableComponent.indeterminate).toBeFalsy();
expect(dataTableComponent.allSelected).toBeFalsy();

fixture.detectChanges();
fixture.whenStable().then(() => {
// select the first and last row with shift key also selected and should then select all checkboxes
let clickEvent: MouseEvent = document.createEvent('MouseEvents');
// the 12th parameter below 'true' sets the shift key to be clicked at the same time as as the mouse click
clickEvent.initMouseEvent('click', true, true, window, 0, 0, 0, 0, 0, false, false, true/*shiftkey*/, false, 0, document.body.parentNode);
fixture.debugElement.queryAll(By.directive(MdPseudoCheckbox))[0].nativeElement.dispatchEvent(clickEvent);
fixture.debugElement.queryAll(By.directive(MdPseudoCheckbox))[3].nativeElement.dispatchEvent(clickEvent);
fixture.detectChanges();
fixture.whenStable().then(() => {
// check to see if allSelected is true
expect(dataTableComponent.allSelected).toBeTruthy();
done();
});
});
});
})();
});

it('should click on a row and see the rowClick Event',
async(inject([], () => {
let fixture: ComponentFixture<any> = TestBed.createComponent(TdDataTableBasicComponent);
let component: TdDataTableBasicComponent = fixture.debugElement.componentInstance;

let eventSpy: jasmine.Spy = spyOn(fixture.debugElement.query(By.directive(TdDataTableComponent)).componentInstance, 'clickRow');

fixture.detectChanges();
fixture.whenStable().then(() => {
fixture.debugElement.queryAll(By.directive(TdDataTableRowComponent))[1].nativeElement.click();
fixture.detectChanges();
fixture.whenStable().then(() => {
expect(eventSpy).toHaveBeenCalled();
});
});
})));
});
});

@Component({
Expand Down
116 changes: 93 additions & 23 deletions src/platform/core/data-table/data-table.component.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Component, Input, Output, EventEmitter, forwardRef, ChangeDetectionStrategy, ChangeDetectorRef,
ContentChildren, TemplateRef, AfterContentInit, QueryList } from '@angular/core';
ContentChildren, TemplateRef, AfterContentInit, QueryList, HostListener } from '@angular/core';
import { NG_VALUE_ACCESSOR, ControlValueAccessor } from '@angular/forms';

import { ITdDataTableSortChangeEvent } from './data-table-column/data-table-column.component';
Expand Down Expand Up @@ -42,6 +42,10 @@ export interface ITdDataTableSelectAllEvent {
selected: boolean;
}

export interface ITdDataTableRowClickEvent {
row: any;
}

@Component({
providers: [ TD_DATA_TABLE_CONTROL_VALUE_ACCESSOR ],
selector: 'td-data-table',
Expand Down Expand Up @@ -71,6 +75,9 @@ export class TdDataTableComponent implements ControlValueAccessor, AfterContentI
private _sortBy: ITdDataTableColumn;
private _sortOrder: TdDataTableSortingOrder = TdDataTableSortingOrder.Ascending;

/** shift select */
private _lastSelectedIndex: number = -1;

/** template fetching support */
private _templateMap: Map<string, TemplateRef<any>> = new Map<string, TemplateRef<any>>();
@ContentChildren(TdDataTableTemplateDirective) _templates: QueryList<TdDataTableTemplateDirective>;
Expand Down Expand Up @@ -247,6 +254,13 @@ export class TdDataTableComponent implements ControlValueAccessor, AfterContentI
*/
@Output('rowSelect') onRowSelect: EventEmitter<ITdDataTableSelectEvent> = new EventEmitter<ITdDataTableSelectEvent>();

/**
* onRowClick?: function
* Event emitted when a row is clicked.
* Emits an [ITdDataTableRowClickEvent] implemented object.
*/
@Output('rowClick') onRowClick: EventEmitter<ITdDataTableRowClickEvent> = new EventEmitter<ITdDataTableRowClickEvent>();

/**
* selectAll?: function
* Event emitted when all rows are selected/deselected by the all checkbox. [selectable] needs to be enabled.
Expand Down Expand Up @@ -333,32 +347,66 @@ export class TdDataTableComponent implements ControlValueAccessor, AfterContentI
}

/**
* Selects or clears a row depending on 'checked' value
* Selects or clears a row depending on 'checked' value if the row 'isSelectable'
* handles cntrl clicks and shift clicks for multi-select
*/
select(row: any, checked: boolean, event: Event): void {
event.preventDefault();
// clears all the fields for the dataset
if (!this._multiple) {
this.clearModel();
}

if (checked) {
this._value.push(row);
} else {
// if selection is done by a [uniqueId] it uses it to compare, else it compares by reference.
if (this.uniqueId) {
row = this._value.filter((val: any) => {
return val[this.uniqueId] === row[this.uniqueId];
})[0];
if (this.isSelectable) {
event.preventDefault();
// clears all the fields for the dataset
if (!this._multiple) {
this.clearModel();
}
let index: number = this._value.indexOf(row);
if (index > -1) {
this._value.splice(index, 1);
let currentSelected: number = this._data.findIndex((d: any) => d === row);
this._doSelection(row);

// Check to see if Shift key is selected and need to select everything in between
let mouseEvent: MouseEvent = event as MouseEvent;
if (this.isMultiple && mouseEvent && mouseEvent.shiftKey && this._lastSelectedIndex > -1) {
let firstSelected: number = this._data.findIndex((d: any) => this.isRowSelected(d));
let lastSelected: number = this._data.concat([]).reverse().findIndex((d: any) => this.isRowSelected(d));
// find the index when not reversed
lastSelected = (this._data.length - 1) - lastSelected;
if (firstSelected > -1 && lastSelected > -1) {
for (let i: number = firstSelected; i < lastSelected; i++) {
if (this._data[i] !== row && i !== this._lastSelectedIndex) {
this._doSelection(this._data[i]);
}
}
}
}
this._lastSelectedIndex = currentSelected;
}
}

/**
* Overrides the onselectstart method of the document so other text on the page
* doesn't get selected when doing shift selections.
*/
@HostListener('window:mousedown', ['$event'])
disableOnSelectStart(): void {
if (event.srcElement.tagName === 'MD-PSEUDO-CHECKBOX') {
document.onselectstart = function(): boolean {
return false;
};
}
this._calculateCheckboxState();
this.onRowSelect.emit({row: row, selected: checked});
this.onChange(this._value);
}

/**
* Resets the original onselectstart method.
*/
@HostListener('window:mouseup', ['$event'])
reEnableOnSelectStart(): void {
if (event.srcElement.tagName === 'MD-PSEUDO-CHECKBOX') {
Copy link
Contributor

@emoralesb05 emoralesb05 May 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

document.onselectstart = undefined;
}
}

/**
* emits the onRowClickEvent when a row is clicked
*/
clickRow(row: any): void {
this.onRowClick.emit({row: row});
}

/**
Expand Down Expand Up @@ -405,6 +453,29 @@ export class TdDataTableComponent implements ControlValueAccessor, AfterContentI
}
}

/**
* Does the actual Row Selection
*/
private _doSelection(row: any): void {
if (!this.isRowSelected(row)) {
this._value.push(row);
} else {
// if selection is done by a [uniqueId] it uses it to compare, else it compares by reference.
if (this.uniqueId) {
row = this._value.filter((val: any) => {
return val[this.uniqueId] === row[this.uniqueId];
})[0];
}
let index: number = this._value.indexOf(row);
if (index > -1) {
this._value.splice(index, 1);
}
}
this._calculateCheckboxState();
this.onRowSelect.emit({row: row, selected: this.isRowSelected(row)});
this.onChange(this._value);
}

/**
* Calculate all the state of all checkboxes
*/
Expand Down Expand Up @@ -436,5 +507,4 @@ export class TdDataTableComponent implements ControlValueAccessor, AfterContentI
}
}
}

}