-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(paginator): update page index if invalid after length change #14625
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
base: main
Are you sure you want to change the base?
Conversation
src/lib/paginator/paginator.ts
Outdated
const currentPageIndex = this._pageIndex; | ||
|
||
if (currentPageIndex > maxPageIndex && this.initialized) { | ||
// Needs to happen on the next tick, in order to avoid "changed after checked" errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this cause a changed after checked error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's been a while so I don't remember the exact reason. I think it had to do with the page event being emitted at the wrong time the which would cause the table to be re-rendered after the changes were detected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel iffy about just throwing a microtask at it, especially without a clear explanation of why it's necessary.
534af71
to
9febcbe
Compare
9febcbe
to
1b3c9f8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Little concerned about timing on this - could this interfere if the user tries to change the index and length at the same time? E.g. if the user changes the index + length, and the length gets set first, then a promise gets started that will change the index even though the user will have already changed it at that point
I've been looking at @andrewseguin 's comments and I think that concern would be addressed by moving L136 to L130 (and L139 to L141, so that all the new code happens inside of the |
@crisbeto it looks like your PR has fallen out of date please rebase the PR. |
…ange Updates the page index of a paginator if its length changed to something where the index isn't valid anymore (e.g. the user is on the last page and the amount of pages becomes smaller). Fixes angular#14520.
I've rebased the PR. |
1b3c9f8
to
339c2eb
Compare
Updates the page index of a paginator if its length changed to something where the index isn't valid anymore (e.g. the user is on the last page and the amount of pages becomes smaller).
Fixes #14520.