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

NzTableComponent should support CDK Datasource #3571

Open
lppedd opened this issue Jun 14, 2019 · 15 comments
Open

NzTableComponent should support CDK Datasource #3571

lppedd opened this issue Jun 14, 2019 · 15 comments

Comments

@lppedd
Copy link
Contributor

lppedd commented Jun 14, 2019

What problem does this feature solve?

NzTableComponent will be able to support virtual scrolling of unknown size data sources.
Currently, unlike NzListComponent, the table component supports only a static array as input, even for virtual scrolling. This limits the table capabilities, as it uses the Angular CDK virtual scroll viewport, which supports a Datasource implementation natively (@angular/cdk/collections).

<ng-container *cdkVirtualFor="let item of data; let i = index">

Custom solutions do indeed work. However they work poorly, especially in respect to change detection and scrolling behavior.

What does the proposed API look like?

NzTableComponent should accept nzData: T[] | Datasource<T>;.

@lppedd
Copy link
Contributor Author

lppedd commented Jun 14, 2019

If you haven't got time for this, I can consider a PR, although I can't guarantee on the expected delivery time. Also, I think there is a design choice to make, as of now the code does strictly target a simple array.

@vthinkxie
Copy link
Member

Hi @lppedd
The idea sounds great. Does this Datasource<T> support only virtual scroll or both virtual scroll and normal mode?

@lppedd
Copy link
Contributor Author

lppedd commented Jun 14, 2019

@vthinkxie The Datasource should support only virtual scrolling by default AFAIK.
Its API does state it accepts a CollectionViewer (source), which is implemented and injected by the cdkVirtualFor directive.

As you can see CollectionViewer does only have a viewChange: Observable<ListRange>; field.
Thus, a custom implementation could be created to support a fixed size array (the viewChange observable could emit on page changes), in conjuction with a custom, hypothetical ArrayDatasource, maintained internally the NzTableComponent. This will prevent having to modify the public table API too much.

This request comes from the necessity to display an "audit" of user operations, thus this is potentially infinite going backwards, and while paging is still an option, an infinite scroll is a lot better for usability.

@vthinkxie
Copy link
Member

@lppedd I got it, just feel free to submit PR for this, we will review it, thanks a lot for your proposal

@lppedd
Copy link
Contributor Author

lppedd commented Jun 14, 2019

@vthinkxie do you prefer maintaining the array/datasource as two separate cases, or do you feel like an homogeneous implementation would be better? The first one would touch only the virtual scroll scenario, while the second would be an overall refactor I think (but I'll have to look at the code better)

@vthinkxie
Copy link
Member

I think both solutions are ok, but it has to stay compatible with previous version

@lppedd
Copy link
Contributor Author

lppedd commented Jun 28, 2019

@vthinkxie thanks! I will open the PR shortly.

@lppedd
Copy link
Contributor Author

lppedd commented Jul 10, 2019

I'm now working on this full-time at work.
I'm battling with, apparently, a change-detection issue, caused by the multiple templates nesting & the CdkVirtualForOf directive itself.
The view is constantly re-rendered, causing an odd infinite-automatic-scroll behavior.

The only solution seems to be detaching from change-detection and detectChanges manually on scrolling, e.g.

this.cdkVirtualScrollViewport
  .elementScrolled()
  .subscribe({ next: () => this.cdr.detectChanges() });

Alternatively, the cdk-virtual-scroll-viewport must be placed under a non-conditional template (tbc).

@lppedd
Copy link
Contributor Author

lppedd commented Jul 10, 2019

@vthinkxie repro here, maybe you already saw something like that.
Just set TableDataSource#pageSize to 10 and scroll down the list.
Btw, this affects also the NzListComponent, and it appears when the page size renders less items then what the total height of the viewport can render.

@vthinkxie
Copy link
Member

@lppedd thanks to your efforts!
we will check it later

@lppedd
Copy link
Contributor Author

lppedd commented Jul 11, 2019

@vthinkxie I've done a bit of refactoring.
I'll now continue to post update on #3766.

@hsuanxyz hsuanxyz added this to the backlog milestone Oct 22, 2020
@fattikus
Copy link

any updates on this? or it was scrapped

@lppedd
Copy link
Contributor Author

lppedd commented Feb 12, 2021

@fattikus unfortunately I don't work anymore on Angular, and I don't have an environment to develop this. But with recent API changes it should be easier for you to complete.

@borjaj14
Copy link

Any update on this???
I think it would be a nice feature...

Nowadays there is no other alternative to implementing the virtual scroll in zorro's table?

@lppedd
Copy link
Contributor Author

lppedd commented Mar 14, 2022

@borjaj14 hi! You could try to finish this PR. My initial implementation was architecturally fine as far as I remember, it should be pretty easy to understand and pick it up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants