fix(paginator): not emitting page event when pageIndex, pageSize or length changes
Currently the paginator won't fire the page event if it's pageIndex, length or pageSize change, which means that the associated table won't be able to react. These changes emit the event if the values change.
Fixes #12583.
Something does feel weird about this change. Andrew would have a better idea, but I vaguely recall the decision for page events to emit on user interaction being intentional. The thinking was probably that if the user is setting new values into the paginator, they obviously already know about those values and should also propagate them to wherever else they're needed. This makes the page event consistent with change events from other components, which also do this, mirroring native html controls.
I suppose the difference between the page event and the various change events is that page is also being used internally inside the table data source in order to know when to re-render the data.
Mind if we sit on this one for a few weeks until Andrew gets back? I'd like to chat with him about it.
Hi @crisbeto! This PR has merge conflicts due to recent upstream merges. Please help to unblock it by resolving these conflicts. Thanks!
The page index is typically set to zero in user code in response to sort changes and filter changes, which typically themselves already result in table-reloading chains of events including calls to a service or repository. (While an application-user might be half-way through a long paginated list, they are generally assumed to want to be placed at the top of the reformulated list when the sort or filter changes appear. Sort and filter changes render page position meaningless or at least ambiguous; and, by convention, sorting is seldom implemented on a paginated-page-local basis.) If MatPaginator.page fires as a result of this reset in code to the first page, a second, redundant call on the datasource would be caused under common circumstances, unless user code implements some sort of gating or temporal windowing/debouncing mechanism.
Overall, this change might cause some inconvenience for alert developers, and, a subtle bandwidth and performance loss for less-alert developers. (Subtle, because it will be triggered only if the page number is not already zero at the time the filter or sort change is made; we are talking about an extra, back-end call in SPAs; and, finally, it might not jump off the page for some devs less familiar with asynchronous reactive subscription lambda syntax.) If this breaking change is to be made, then perhaps it should be mitigated by adding a new MatPaginator method that goes to the first page without triggering the MatPaginator.page event and to disseminate knowledge (e.g. in the examples) that this call should be used in lieu of this.paginator.pageIndex = 0 in this scenario.
For typical current user code, consider the following excerpt from the "Table retrieving data through HTTP" mat-table example in the API docs online.
ngOnInit() {
this.exampleDatabase = new ExampleHttpDao(this.http);
// If the user changes the sort order, reset back to the first page.
this.sort.sortChange.subscribe(() => **this.paginator.pageIndex = 0**);
merge(**this.sort.sortChange, this.paginator.page**)
.pipe(
startWith({}),
switchMap(() => {
this.isLoadingResults = true;
return this.exampleDatabase!.getRepoIssues(
this.sort.active, this.sort.direction, this.paginator.pageIndex);
}),
map(data => {...
(Incidentally, I found this issue when I sought to confirm that, as things stand, I would not have to intercept/block incident calls to my datasource<T> indirectly caused by my component filtering and sort event-handling functions.)
Any updates on this?