taiga-ui icon indicating copy to clipboard operation
taiga-ui copied to clipboard

🐞 - table-pagination - sizeChange souldn't call pageChange

Open xvs32x opened this issue 2 years ago • 3 comments

Which @taiga-ui/* package(s) are the source of the bug?

addon-table

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/taiga-starter

Is this issue blocking you?

Blocking

Description

Hi there, at first, thank you for amazing UI Library. It's the greatest! A few days ago I decided to use TaigaUI for my project and met a proplem.

Screenshot 2022-07-04 160929

In my application router query params and pagination are synced, so query string is a source of truth for the pagination. It looks like when a user change something on the page (pageIndex, perPage, etc), at first, I use router to change the query string, and then, state from query string changes the pagination on a page. Unfortunately, it doesn't work well, because when I change page size (Dropdown on the picture), table-pagination unexpectedly calls pageChange event and router.navigate is called twice (For page size changing and then for current page changing). But angular can't handle it (two navigates an once) and I get an error router.mjs:5931 Router Event: NavigationCancel router.mjs:5932 NavigationCancel(id: 6, url: '/crud?page=1&per_page=20') router.mjs:5933 NavigationCancel {id: 6, url: '/crud?page=1&per_page=20', reason: 'Navigation ID 6 is not equal to the current navigation id 7'} It means that angular can't update query params, because another navigation has started.

I think it happens here. As we can see there are two calls at once.

this.sizeChange.emit(size); this.pageChange.emit(this.page);

In my opinion this behavior is not trasparent and I'd like to have an ability to change the page on my own with page size (at once) to prevent this error.

Thank you!

Angular version

13.2.0

Taiga UI version

2.50.0

Which browsers have you used?

  • [X] Chrome
  • [ ] Firefox
  • [ ] Safari
  • [ ] Edge

Which operating systems have you used?

  • [ ] macOS
  • [X] Windows
  • [ ] Linux
  • [ ] iOS
  • [ ] Android

xvs32x avatar Jul 04 '22 13:07 xvs32x

@xvs32x hello, could you please create a minimal reproduce on stackblitz or github?

splincode avatar Jul 08 '22 14:07 splincode

@xvs32x hello, could you please create a minimal reproduce on stackblitz or github?

https://stackblitz.com/edit/taiga-starter-75yzze

Query parameters should be updated when we change page size (per_page param should be added). But it doesn't happen, because another navigation starts. We can check it if comment the line inside changePage method

xvs32x avatar Jul 13 '22 09:07 xvs32x

Think @xvs32x is right about this unexpected behavior. But I can understand what it for by default

Couple ways to solve it:

  • add new param for pagination component, like a pageRecalc = true to prevent calculations at this lines if necessary:

https://github.com/Tinkoff/taiga-ui/blob/edd1da8cf1bfe3eca2e96f5fed51df693e91f2b5/projects/addon-table/components/table-pagination/table-pagination.component.ts#L76-L77

  • change behavior totally to material-way, where pageChange and sizeChange are single typed event:
  /** Emits an event notifying that a change of the paginator's properties has been triggered. */
  private _emitPageEvent(previousPageIndex: number) {
    this.page.emit({
      previousPageIndex,
      pageIndex: this.pageIndex,
      pageSize: this.pageSize,
      length: this.length,
    });
  }

Now-fix for @xvs32x – try rxjs throttle

If first solution is suitable I can take this issue on the weekdays

zlodeev avatar Jul 16 '22 21:07 zlodeev

@splincode Hi, Can I create PR to help you fix it?

xvs32x avatar Oct 12 '22 12:10 xvs32x

@xvs32x yes, of course

splincode avatar Oct 12 '22 13:10 splincode