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

Debounce the layout function in the list component #1928

4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- `textfield`
- `autoValidate` now validates on value change rather than input

- `list`
- List items removal from DOM initiates an async layout in the managing list
- Added `itemsReady` promise to the list's updateComplete

## [v0.19.1] - 2020-10-08

### Fixed
Expand Down
44 changes: 43 additions & 1 deletion packages/list/mwc-list-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,28 @@ import {Layoutable, ListItemBase, RequestSelectedDetail} from './mwc-list-item-b

export {ActionDetail, createSetFromIndex, isEventMulti, isIndexSet, MWCListIndex, SelectedDetail} from './mwc-list-foundation';

function debounceLayout(
callback: (updateItems: boolean) => void, waitInMS = 50) {
let timeoutId: number;
return function(updateItems = true) {
clearTimeout(timeoutId);
timeoutId =
setTimeout(() => callback(updateItems), waitInMS) as unknown as number;
};
}

const isListItem = (element: Element): element is ListItemBase => {
return element.hasAttribute('mwc-list-item');
};

function clearAndCreateItemsReadyPromise(this: ListBase) {
const oldResolver = this.itemsReadyResolver;
this.itemsReady = new Promise((res) => {
return this.itemsReadyResolver = res;
});
oldResolver();
}

/**
* @fires selected {SelectedDetail}
* @fires action {ActionDetail}
Expand All @@ -59,7 +76,6 @@ export abstract class ListBase extends BaseElement implements Layoutable {
})
activatable = false;


@property({type: Boolean})
@observer(function(this: ListBase, newValue: boolean, oldValue: boolean) {
if (this.mdcFoundation) {
Expand Down Expand Up @@ -113,6 +129,30 @@ export abstract class ListBase extends BaseElement implements Layoutable {
})
noninteractive = false;

debouncedLayout: (updateItems?: boolean) => void | undefined;
YonatanKra marked this conversation as resolved.
Show resolved Hide resolved
protected itemsReadyResolver:
(value?: (PromiseLike<never[]>|never[]|undefined)) => void =
(() => {
//
}) as(value?: (PromiseLike<unknown[]>|unknown[])) => void;

constructor() {
super();
const debouncedFunction = debounceLayout(this.layout.bind(this));
this.debouncedLayout = (updateItems = true) => {
clearAndCreateItemsReadyPromise.call(this);

debouncedFunction(updateItems);
};
}

itemsReady = Promise.resolve([]);

async _getUpdateComplete() {
await super._getUpdateComplete();
await this.itemsReady;
}

protected get assignedElements(): Element[] {
const slot = this.slotElement;

Expand Down Expand Up @@ -486,6 +526,8 @@ export abstract class ListBase extends BaseElement implements Layoutable {
first.tabindex = 0;
}
}

this.itemsReadyResolver();
}

getFocusedItemIndex() {
Expand Down
5 changes: 4 additions & 1 deletion packages/list/mwc-list-item-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export interface RequestSelectedDetail {

export interface Layoutable {
layout: (updateItems?: boolean) => void;
debouncedLayout?: (updateItems?: boolean) => void | undefined;
}

export type GraphicType = 'avatar'|'icon'|'medium'|'large'|'control'|null;
Expand Down Expand Up @@ -277,7 +278,9 @@ export class ListItemBase extends LitElement {
}

if (this._managingList) {
this._managingList.layout(true);
this._managingList.debouncedLayout ?
this._managingList.debouncedLayout(true) :
this._managingList.layout(true);
}
}

Expand Down
29 changes: 28 additions & 1 deletion packages/list/test/mwc-list.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ const createEmptyList = (emptyMessage: string|undefined) => {
};

suite('mwc-list:', () => {
let fixt: TestFixture;
let fixt: TestFixture|null;

suite('mwc-list-item', () => {
suite('initialization', () => {
Expand Down Expand Up @@ -1788,5 +1788,32 @@ suite('mwc-list:', () => {
}
});
});

suite('performance issue', () => {
test(
'removing a list should not call layout more than once', async () => {
let count = 0;
const originalLayout = List.prototype.layout;
List.prototype.layout = function(update) {
originalLayout.call(this, update);
count++;
};

const itemsTemplates = new Array(300).fill(0).map(() => listItem());
YonatanKra marked this conversation as resolved.
Show resolved Hide resolved
fixt = await fixture(listTemplate({items: itemsTemplates}));
element = fixt.root.querySelector('mwc-list')!;

count = 0;

fixt.remove();
await element.updateComplete;
fixt = null;
assert.equal(
count,
1,
'list.layout ran more than once while it shouldn\'t have');
List.prototype.layout = originalLayout;
});
});
});
});