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): virtual scroll + sticky headers + column widths #859

Merged
merged 25 commits into from
Sep 11, 2017

Conversation

emoralesb05
Copy link
Contributor

@emoralesb05 emoralesb05 commented Sep 6, 2017

Description

What's included?

  • feat(data-table): add [td-data-table-column-row] atomic element to be more explicit on data-table structure.
  • feat(data-table): add virtual scroll capability when setting flex or a static height
  • feat(data-table): add sticky headers capability when setting flex or a static height
  • feat(data-table): add configurable column widhts via ITdDataTableColumn config
  • fix(data-table): issue when changing pages and unselecting all rows Td-data-table #808
  • Updated DEMO and DOCS.

Test Steps

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.
Screenshots or link to CodePen/Plunker/JSfiddle

This was referenced Sep 8, 2017
sortable?: boolean; // used to make a particular column sortable
hidden?: boolean; // used to hide the column in the data table on the fly
filter?: boolean;
width?: ITdDataTableColumnWidth | number; // used to configure the widhts of the columns, if omitted it will fill the rest of the space
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo, widhts

tooltip?: string; // used to add a tooltip into the column header
numeric?: boolean; // used to right align elements if they are numeric
format?: (value: any) => any; // used to format the cell values
nested?: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't you add a description on this one also?

nested?: boolean;
sortable?: boolean; // used to make a particular column sortable
hidden?: boolean; // used to hide the column in the data table on the fly
filter?: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't you add a description on this one also?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one is mute.. i actually debated about removing it since its not used in the data table.. its used as a helper property outside of it.

import { TdDataTableColumnComponent } from '../data-table-column/data-table-column.component';

@Component({
/* tslint:disable-next-line */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this disable-next-line necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup, since component typically need to start with td haha and only be selectors

<tr td-data-table-row
#dtRow
[tabIndex]="selectable ? 0 : -1"
[selected]="(clickable || selectable) && isRowSelected(row)"
*ngFor="let row of data; let rowIndex = index"
*ngFor="let row of data | slice:fromRow:toRow; let rowIndex = index"
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice 🎩

}
this._shiftPreviouslyPressed = true;
} else if (mouseEvent && !mouseEvent.shiftKey) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

comments on to what is going on here?

if (fixedTotalWidth < this.hostWidth) {
recalculateHostWidth = this.hostWidth - fixedTotalWidth;
}
if (flexibleWidths && recalculateHostWidth) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

lots of stuff going on in here, comments would be useful

if (widthOpts && widthOpts.min >= this._widths[index].value) {
this._widths[index].value = widthOpts.min;
this._widths[index].min = true;
} else if (widthOpts && widthOpts.max <= this._widths[index].value) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above, comments would help

@@ -92,8 +92,11 @@ export class TdDataTableColumnComponent {
this._renderer.addClass(this._elementRef.nativeElement, 'td-data-table-column');
}

handleSortBy(): void {
this.onSortChange.emit({name: this.name, order: this._sortOrder});
@HostListener('click', ['event'])
Copy link
Collaborator

Choose a reason for hiding this comment

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

add some comments of to why this is needed

<th td-data-table-column class="mat-checkbox-column" *ngIf="selectable">
<md-checkbox
#checkBoxAll
*ngIf="multiple"
[disabled]="!hasData"
[indeterminate]="indeterminate && !allSelected && hasData"
[checked]="allSelected && hasData"
(click)="selectAll(!checkBoxAll.checked)"
(click)="blockEvent($event); selectAll(!checkBoxAll.checked)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm, can we add comments here to why this is blocked?

constructor(private _elementRef: ElementRef, private _renderer: Renderer2) {
this._renderer.addClass(this._elementRef.nativeElement, 'td-data-table-row');
}

@HostListener('click', ['$event'])
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we add comments on to what this is here for?

}

get offsetTransform(): SafeStyle {
let offset: number = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

comments here?

@emoralesb05
Copy link
Contributor Author

Updated code with comments on how a few things work

@jeremysmartt jeremysmartt self-assigned this Sep 11, 2017
@emoralesb05 emoralesb05 merged commit c5f065a into develop Sep 11, 2017
@emoralesb05 emoralesb05 deleted the feature/data-table-v2 branch September 11, 2017 18:26
@weijyewang
Copy link

hi, is the documentation updated to show on the sticky header and virtual scrolling usage?

@emoralesb05
Copy link
Contributor Author

The 3rd demo uses both in our docs site.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants