components icon indicating copy to clipboard operation
components copied to clipboard

feat(cdk/table): virtual scroll directive for tables

Open MichaelJamesParsons opened this issue 5 years ago • 6 comments

Adds a virtualRows directive which enables virtual scrolling for CDK tables.

Open questions

Blocks of code that are subject to further discussion or contain potentially breaking changes are annotated with FIXME.

Out of scope for now

  • Variable size rows (virtual scroll autosize directive)
  • Material tables support (let's hash out the CDK variant first)
  • Checkbox state
  • Row animations

Known issues

  • Sticky headers/footers flicker if an animation frame exceeds ~35ms. Karl and I will investigate further.

Performance considerations

We've already improved row rendering by 50% or better. Without headers/footers, the scrolling experience is comparable to most other Angular material virtual scroll demos.

However, the entire table shifts position as the user scrolls. The position of sticky headers and footers must be offset by the distance scrolled to keep them in place. On average, the table is able to keep 15-20ms per animation frame. Frames that occasionally exceed ~35ms cause sticky headers/footers to flicker. It looks like longer than usual style calculations are hindering performance the most.

While I believe we can further optimize rendering, we will likely reach a threshold where the table can no longer be optimized and non-trivial tables will still experience flickering (e.g. tables with inline editing, cells with custom components, etc.). A while back, I experimented with rendering tables in multiple pieces so the body content can scroll independently of header/footers (https://github.com/angular/components/pull/20414). There are some a11y challenges to this solution, but it would resolve the flickering.

For example:

<table><!-- table header --></table>
<div class="scroll-viewport">
  <table>
    <thead><!-- visually hidden header --></thead>
    <tbody>...</tbody>
    <tfoot><!--visually hidden footer --></tfoot>
</div>
<table><!-- table footer --></table>

Related PRs

  • https://github.com/angular/components/pull/21704

MichaelJamesParsons avatar Jan 26 '21 22:01 MichaelJamesParsons

Since the description mentions an issue with flickering, could it be the same problem as https://github.com/angular/components/issues/21576?

crisbeto avatar Jan 27 '21 19:01 crisbeto

Since the description mentions an issue with flickering, could it be the same problem as #21576?

Yes, though the flickering I'm seeing is not nearly as significant as observed in #21576. Likely due to internal performance optimizations that are present in this PR, but not yet accessible through the public API (e.g. recycle view repeater and enhancements made in #21704).

MichaelJamesParsons avatar Jan 27 '21 20:01 MichaelJamesParsons

@jelbourn The view_engine_build check fails with the following error, but passes locally.

ERROR: /home/circleci/ng/src/dev-app/table/BUILD.bazel:5:10: Compiling Angular templates (ViewEngine - devmode) //src/dev-app/table:table failed
RangeError: Maximum call stack size exceeded

Local command:

 bazel build --build_tag_filters=-docs-package,-release-package --config=view-engine -- src/...

Am I missing something?

MichaelJamesParsons avatar Jun 30 '21 06:06 MichaelJamesParsons

I resolved the view engine failure described in https://github.com/angular/components/pull/21708#issuecomment-871123887 by adding a missing build target to the dev-app/BUILD.bazel file. I'm surprised the dev-app was able to build at all without it. Strange.

@jelbourn Ready for another review.

MichaelJamesParsons avatar Aug 10 '21 20:08 MichaelJamesParsons

@MichaelJamesParsons Would you like to rebase and continue iterating on this?

andrewseguin avatar Mar 24 '22 19:03 andrewseguin

Yes, I'll set aside time to wrap this up.

MichaelJamesParsons avatar Mar 29 '22 17:03 MichaelJamesParsons