Skip to content

Commit

Permalink
fix(data-table): Set progress indicator height to table body offset h…
Browse files Browse the repository at this point in the history
…eight

Fixes progress indicator height to cover only table body height (excluding scroll content height).

BREAKING CHANGE: New adapter method replacing `getTableBodyHeight()` => `getTableHeaderHeight()` and changed return types of this method.

PiperOrigin-RevId: 318845959
  • Loading branch information
abhiomkar authored and copybara-github committed Jun 29, 2020
1 parent c1fec42 commit c678a9d
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 20 deletions.
6 changes: 3 additions & 3 deletions packages/mdc-data-table/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,14 +211,14 @@ export interface MDCDataTableAdapter {
notifySortAction(data: SortActionEventDetail): void;

/**
* @return Returns computed styles height of table's body element.
* @return Returns computed styles height of table container element.
*/
getTableBodyHeight(): string;
getTableContainerHeight(): number;

/**
* @return Returns computed styles height of table's header element.
*/
getTableHeaderHeight(): string;
getTableHeaderHeight(): number;

/**
* Sets progress indicator CSS styles to position it on top of table body.
Expand Down
14 changes: 7 additions & 7 deletions packages/mdc-data-table/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,15 +151,15 @@ export class MDCDataTable extends MDCComponent<MDCDataTableFoundation> {
notifySortAction: (data) => {
this.emit(events.SORTED, data, /** shouldBubble */ true);
},
getTableBodyHeight: () => {
const tableBody =
this.root.querySelector<HTMLElement>(selectors.CONTENT);
getTableContainerHeight: () => {
const tableContainer =
this.root.querySelector<HTMLElement>(`.${cssClasses.CONTAINER}`);

if (!tableBody) {
throw new Error('MDCDataTable: Table body element not found.');
if (!tableContainer) {
throw new Error('MDCDataTable: Table container element not found.');
}

return `${tableBody.getBoundingClientRect().height}px`;
return tableContainer.getBoundingClientRect().height;
},
getTableHeaderHeight: () => {
const tableHeader =
Expand All @@ -169,7 +169,7 @@ export class MDCDataTable extends MDCComponent<MDCDataTableFoundation> {
throw new Error('MDCDataTable: Table header element not found.');
}

return `${tableHeader.getBoundingClientRect().height}px`;
return tableHeader.getBoundingClientRect().height;
},
setProgressIndicatorStyles: (styles) => {
const progressIndicator =
Expand Down
1 change: 1 addition & 0 deletions packages/mdc-data-table/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
export const cssClasses = {
CELL: 'mdc-data-table__cell',
CELL_NUMERIC: 'mdc-data-table__cell--numeric',
CONTAINER: 'mdc-data-table__container',
CONTENT: 'mdc-data-table__content',
HEADER_CELL: 'mdc-data-table__header-cell',
HEADER_CELL_LABEL: 'mdc-data-table__header-cell-label',
Expand Down
15 changes: 9 additions & 6 deletions packages/mdc-data-table/foundation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ export class MDCDataTableFoundation extends MDCFoundation<MDCDataTableAdapter> {
getRowIdAtIndex: () => '',
getRowIndexByChildElement: () => 0,
getSelectedRowCount: () => 0,
getTableBodyHeight: () => '',
getTableHeaderHeight: () => '',
getTableContainerHeight: () => 0,
getTableHeaderHeight: () => 0,
isCheckboxAtRowIndexChecked: () => false,
isHeaderRowCheckboxChecked: () => false,
isRowsSelectable: () => false,
Expand Down Expand Up @@ -262,12 +262,15 @@ export class MDCDataTableFoundation extends MDCFoundation<MDCDataTableAdapter> {
* loading state.
*/
showProgress() {
const height = this.adapter.getTableBodyHeight();
const top = this.adapter.getTableHeaderHeight();
const tableHeaderHeight = this.adapter.getTableHeaderHeight();
// Calculate the height of table content (Not scroll content) excluding
// header row height.
const height = this.adapter.getTableContainerHeight() - tableHeaderHeight;
const top = tableHeaderHeight;

this.adapter.setProgressIndicatorStyles({
height,
top,
height: `${height}px`,
top: `${top}px`,
});
this.adapter.addClass(cssClasses.IN_PROGRESS);
}
Expand Down
8 changes: 4 additions & 4 deletions packages/mdc-data-table/test/foundation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe('MDCDataTableFoundation', () => {
'getRowIdAtIndex',
'getRowIndexByChildElement',
'getSelectedRowCount',
'getTableBodyHeight',
'getTableContainerHeight',
'getTableHeaderHeight',
'isCheckboxAtRowIndexChecked',
'isHeaderRowCheckboxChecked',
Expand Down Expand Up @@ -484,13 +484,13 @@ describe('MDCDataTableFoundation', () => {
it('#showProgress Adds class name that makes the progress indicator visibile',
() => {
const {foundation, mockAdapter} = setupTest();
mockAdapter.getTableHeaderHeight.and.returnValue('20px');
mockAdapter.getTableBodyHeight.and.returnValue('100px');
mockAdapter.getTableHeaderHeight.and.returnValue(20);
mockAdapter.getTableContainerHeight.and.returnValue(100);

foundation.showProgress();

expect(mockAdapter.setProgressIndicatorStyles)
.toHaveBeenCalledWith({height: '100px', top: '20px'});
.toHaveBeenCalledWith({height: '80px', top: '20px'});
expect(mockAdapter.addClass)
.toHaveBeenCalledWith(cssClasses.IN_PROGRESS);
});
Expand Down

0 comments on commit c678a9d

Please sign in to comment.