Skip to content

perf(table): Slightly improve speed of adding/remvoing sticky styles #19823

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

Merged
merged 8 commits into from
Aug 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 21 additions & 6 deletions src/cdk/table/sticky-styler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export type StickyDirection = 'top' | 'bottom' | 'left' | 'right';
*/
export const STICKY_DIRECTIONS: StickyDirection[] = ['top', 'bottom', 'left', 'right'];


/**
* Applies and removes sticky positioning styles to the `CdkTable` rows and columns cells.
* @docs-private
Expand All @@ -34,12 +35,16 @@ export class StickyStyler {
* @param direction The directionality context of the table (ltr/rtl); affects column positioning
* by reversing left/right positions.
* @param _isBrowser Whether the table is currently being rendered on the server or the client.
* @param _needsPositionStickyOnElement Whether we need to specify position: sticky on cells
* using inline styles. If false, it is assumed that position: sticky is included in
* the component stylesheet for _stickCellCss.
*/
constructor(private _isNativeHtmlTable: boolean,
private _stickCellCss: string,
public direction: Direction,
private _coalescedStyleScheduler: _CoalescedStyleScheduler,
private _isBrowser = true) { }
private _isBrowser = true,
private readonly _needsPositionStickyOnElement = true) { }

/**
* Clears the sticky positioning styles from the row and its cells by resetting the `position`
Expand Down Expand Up @@ -204,13 +209,21 @@ export class StickyStyler {
for (const dir of stickyDirections) {
element.style[dir] = '';
}
element.style.zIndex = this._getCalculatedZIndex(element);

// If the element no longer has any more sticky directions, remove sticky positioning and
// the sticky CSS class.
const hasDirection = STICKY_DIRECTIONS.some(dir => !!element.style[dir]);
if (!hasDirection) {
element.style.position = '';
// Short-circuit checking element.style[dir] for stickyDirections as they
// were already removed above.
const hasDirection = STICKY_DIRECTIONS.some(dir =>
stickyDirections.indexOf(dir) === -1 && element.style[dir]);
if (hasDirection) {
element.style.zIndex = this._getCalculatedZIndex(element);
} else {
// When not hasDirection, _getCalculatedZIndex will always return ''.
element.style.zIndex = '';
if (this._needsPositionStickyOnElement) {
element.style.position = '';
}
element.classList.remove(this._stickCellCss);
}
}
Expand All @@ -223,8 +236,10 @@ export class StickyStyler {
_addStickyStyle(element: HTMLElement, dir: StickyDirection, dirValue: number) {
element.classList.add(this._stickCellCss);
element.style[dir] = `${dirValue}px`;
element.style.cssText += 'position: -webkit-sticky; position: sticky; ';
element.style.zIndex = this._getCalculatedZIndex(element);
if (this._needsPositionStickyOnElement) {
element.style.cssText += 'position: -webkit-sticky; position: sticky; ';
}
}

/**
Expand Down
9 changes: 8 additions & 1 deletion src/cdk/table/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,13 @@ export class CdkTable<T> implements AfterContentChecked, CollectionViewer, OnDes
*/
protected stickyCssClass: string = 'cdk-table-sticky';

/**
* Whether to manually add positon: sticky to all sticky cell elements. Not needed if
* the position is set in a selector associated with the value of stickyCssClass. May be
* overridden by table subclasses
*/
protected needsPositionStickyOnElement = true;

/** Whether the no data row is currently showing anything. */
private _isShowingNoDataRow = false;

Expand Down Expand Up @@ -1119,7 +1126,7 @@ export class CdkTable<T> implements AfterContentChecked, CollectionViewer, OnDes
const direction: Direction = this._dir ? this._dir.value : 'ltr';
this._stickyStyler = new StickyStyler(
this._isNativeHtmlTable, this.stickyCssClass, direction, this._coalescedStyleScheduler,
this._platform.isBrowser);
this._platform.isBrowser, this.needsPositionStickyOnElement);
(this._dir ? this._dir.change : observableOf<Direction>())
.pipe(takeUntil(this._onDestroy))
.subscribe(value => {
Expand Down
1 change: 1 addition & 0 deletions src/material-experimental/mdc-table/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ sass_binary(
deps = [
"//src/material-experimental/mdc-helpers:mdc_helpers_scss_lib",
"//src/material-experimental/mdc-helpers:mdc_scss_deps_lib",
"//src/material/core:core_scss_lib",
],
)

Expand Down
5 changes: 5 additions & 0 deletions src/material-experimental/mdc-table/table.scss
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
@use '../../material/core/style/vendor-prefixes';
@import '@material/data-table/mixins.import';
@import '../mdc-helpers/mdc-helpers';

@include mdc-data-table-core-styles($query: $mat-base-styles-without-animation-query);

.mat-table-sticky {
@include vendor-prefixes.position-sticky;
}
3 changes: 3 additions & 0 deletions src/material-experimental/mdc-table/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ export class MatTable<T> extends CdkTable<T> implements OnInit {
/** Overrides the sticky CSS class set by the `CdkTable`. */
protected stickyCssClass = 'mat-mdc-table-sticky';

/** Overrides the need to add position: sticky on every sticky cell element in `CdkTable`. */
protected needsPositionStickyOnElement = false;

// After ngOnInit, the `CdkTable` has created and inserted the table sections (thead, tbody,
// tfoot). MDC requires the `mdc-data-table__content` class to be added to the body.
ngOnInit() {
Expand Down
5 changes: 5 additions & 0 deletions src/material/core/style/_vendor-prefixes.scss
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,9 @@
-webkit-backface-visibility: $value;
backface-visibility: $value;
}

@mixin position-sticky {
position: -webkit-sticky;
position: sticky;
}
/* stylelint-enable */
1 change: 1 addition & 0 deletions src/material/table/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ sass_library(
sass_binary(
name = "table_scss",
src = "table.scss",
deps = ["//src/material/core:core_scss_lib"],
)

ng_test_library(
Expand Down
6 changes: 6 additions & 0 deletions src/material/table/table.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
@use '../core/style/vendor-prefixes';

$mat-header-row-height: 56px;
$mat-row-height: 48px;
$mat-row-horizontal-padding: 24px;
Expand Down Expand Up @@ -114,3 +116,7 @@ th.mat-header-cell:last-of-type, td.mat-cell:last-of-type, td.mat-footer-cell:la
padding-left: $mat-row-horizontal-padding;
}
}

.mat-table-sticky {
@include vendor-prefixes.position-sticky;
}
3 changes: 3 additions & 0 deletions src/material/table/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,7 @@ import {_DisposeViewRepeaterStrategy, _VIEW_REPEATER_STRATEGY} from '@angular/cd
export class MatTable<T> extends CdkTable<T> {
/** Overrides the sticky CSS class set by the `CdkTable`. */
protected stickyCssClass = 'mat-table-sticky';

/** Overrides the need to add position: sticky on every sticky cell element in `CdkTable`. */
protected needsPositionStickyOnElement = false;
Copy link
Member

Choose a reason for hiding this comment

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

@andrewseguin is there any reason we couldn't just always use a css class for this instead of attaching styles directly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm up for making that change now, though I fear it could be breaking for any projects that have subclassed CdkTable.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it feels to me like an internal implementation detail, but I'm curious to get @andrewseguin's take

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think there's a reason - we should have a follow up PR that attempts to switch to just the class

}
3 changes: 2 additions & 1 deletion tools/public_api_guard/cdk/table.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ export declare class CdkTable<T> implements AfterContentChecked, CollectionViewe
set dataSource(dataSource: CdkTableDataSourceInput<T>);
get multiTemplateDataRows(): boolean;
set multiTemplateDataRows(v: boolean);
protected needsPositionStickyOnElement: boolean;
protected stickyCssClass: string;
get trackBy(): TrackByFunction<T>;
set trackBy(fn: TrackByFunction<T>);
Expand Down Expand Up @@ -315,7 +316,7 @@ export declare type StickyDirection = 'top' | 'bottom' | 'left' | 'right';

export declare class StickyStyler {
direction: Direction;
constructor(_isNativeHtmlTable: boolean, _stickCellCss: string, direction: Direction, _coalescedStyleScheduler: _CoalescedStyleScheduler, _isBrowser?: boolean);
constructor(_isNativeHtmlTable: boolean, _stickCellCss: string, direction: Direction, _coalescedStyleScheduler: _CoalescedStyleScheduler, _isBrowser?: boolean, _needsPositionStickyOnElement?: boolean);
_addStickyStyle(element: HTMLElement, dir: StickyDirection, dirValue: number): void;
_getCalculatedZIndex(element: HTMLElement): string;
_getCellWidths(row: HTMLElement): number[];
Expand Down
1 change: 1 addition & 0 deletions tools/public_api_guard/material/table.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export declare class MatRowDef<T> extends CdkRowDef<T> {
}

export declare class MatTable<T> extends CdkTable<T> {
protected needsPositionStickyOnElement: boolean;
protected stickyCssClass: string;
static ɵcmp: i0.ɵɵComponentDefWithMeta<MatTable<any>, "mat-table, table[mat-table]", ["matTable"], {}, {}, never, ["caption", "colgroup, col"]>;
static ɵfac: i0.ɵɵFactoryDef<MatTable<any>, never>;
Expand Down