Skip to content

Use filter worker in data-table#3808

Merged
balloob merged 6 commits into
devfrom
filter-worker
Sep 30, 2019
Merged

Use filter worker in data-table#3808
balloob merged 6 commits into
devfrom
filter-worker

Conversation

@bramkragten
Copy link
Copy Markdown
Member

@bramkragten bramkragten commented Sep 25, 2019

So the UI stays responsive when we are filtering large amounts of data
Before:
before-worker
After:
after-worker

@bramkragten bramkragten requested a review from balloob September 25, 2019 18:40
@property({ type: String }) private _sortDirection: SortingDirection = null;
private _filteredData: DataTabelRowData[] = [];

private _debounceSearch = debounce(
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'd love to do this without debounce and the 200ms delay, but couldn't get it to work, if you could guide me :-)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we still need this now we also have the other debounce ?

});
const column = { ...columns[sortColumn] };
delete column.template;
return worker.sortData(data, column, direction, sortColumn);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Every time we send data to the worker, we need to copy it all over to the worker. Could we do filtering and sorting in one go ? Basically we get a "slim" column definition just for sorting that we can send over.


private _handleSearchChange(ev: CustomEvent): void {
this._filter = ev.detail.value;
this._debounceSearch(ev);
Copy link
Copy Markdown
Member

@balloob balloob Sep 27, 2019

Choose a reason for hiding this comment

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

We can do something like this:

const startTime = new Date().getTime();
const curRequest = this.curRequest;
this.curRequest++;

const data = await this.worker.filterAndSortData(this.rawData);
if (this.curRequest != curRequest) return;

// optionally, make sure we wait up to 100ms before starting to render
const curTime = new Date().getTime();
const elapsed = curTime - startTime;
if (elapsed < 100) {
  await new Promise(resolve => setTimeout(resolve, 100 - elapsed))
  if (this.curRequest != curRequest) return;
} 

this.data = data;

}

protected updated(properties: PropertyValues) {
protected async updated(properties: PropertyValues) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

updated is a lifecycle method that we override, we cannot change the signature to be async.

}

const clonedColumns: DataTabelColumnContainer = deepClone(this.columns);
Object.values(clonedColumns).forEach((column: DataTabelColumnData) => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we memoize this, so we only create the sorted columns once ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We only do it now if the columns are changed, so memoize would do the same?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ok!

@balloob balloob merged commit 7d09e29 into dev Sep 30, 2019
@delete-merged-branch delete-merged-branch Bot deleted the filter-worker branch September 30, 2019 22:53
@bramkragten bramkragten mentioned this pull request Oct 2, 2019
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants