[w-table] Pagination issue

[DerrikMilligan]

Currently with the w-table pagination if you adjust the pagination.page value a watcher is fired off to re-calculate the pagination values so they're adjusted correctly.

The method that gets called to update everything is updatePaginationConfig, which you can pass an object of data to for internal uses adjusting the page and total it would seem?

There's a section of this code that assigns some variables for shorthand use with some math, but they override the parameter variables. So then this logic if (page) this.goToPage(page) is always true because we've overridden the page variable, so it will attempt to re-load the page again. This causes my setup to loop infinitely, unless it hits 0. Perhaps I'm using this wrong though?

Currently the way I'm attempting to use it like this:

const tablePagination = computed(() => ({
    itemsPerPage: report.value.pagination.rowsPerPage,
    start       : report.value.pagination.currentPage * report.value.pagination.rowsPerPage,
    end         : (report.value.pagination.currentPage * report.value.pagination.rowsPerPage) + report.value.pagination.rowsPerPage,
    // I add one here because the math I had was a 0 based pagination on the server.
    page        : report.value.pagination.currentPage + 1,
    total       : report.value.totalResults,
}));

And for fetching I'm using:

// Again it's -1 because my stuff is 0 indexed
:fetch="({ page }) => gotoPage(page - 1)"

This causes some really weird behavior unless we use the fix I have in this PR. Again, perhaps I'm doing something wonky here. I'm fine if you toss this out. I can always use the slot for what I need.

[antoniandre]

I've just pulled in for now, but I will have to retest the whole logic as I forgot half of it and it seems to lack some code comments 😅 I think I intended to define the variables like this so it would trigger using what is given in param or use the previously defined var to do the same things.

The paginationConfig object is read & write, it's meant to be updated once on init then monitored with the following watchers, and whatever changes will update the rest of the variables accordingly and trigger necessary actions.

    'pagination.page' (page) {
      this.updatePaginationConfig({ page })
    },
    'pagination.itemsPerPage' (itemsPerPage) {
      this.updatePaginationConfig({ itemsPerPage })
    },
    'pagination.total' (total) {
      this.updatePaginationConfig({ total })
    }

[DerrikMilligan]

That makes sense. Again maybe my use case is odd and shouldn't be the only driving factor.

I'm using a table with filters that make server side API calls to update the data. And when I was using the :fetch method to pull the next page, I was updating my pagination variables as it pulls back all that information along with the rows. I was re-using the same method to pull the data when the filters change, which could change how many pages there are. So I was updating it from the outside and running into a weird infinite loop. My pagination system was 0 based as well so maybe that's why it was so weird.

I am okay if you want to roll this back for now. It's not something mission critical for me and maybe my odd circumstances are at fault here and not the underlying method.

[antoniandre]

I'll dig further a bit and at least fill up all the code comments needed :) What I see for now is that the original overriding is useful for the case (visible in the documentation > client side pagination, for instance) where you first navigate to page 2, then change the items per page to 100: this should be now displaying the rows 101 to 200. But with this change it would show results 1-200. It's definitely useful to review that because it's not a straightforward code to be honest and also there's another issue about the pagination with server side #135, and maybe your case is related.

[antoniandre]

For now I will revert this change. If you could share a reproduction to the problem that will definitely help understanding and fixing whatever is not working properly :) For now I just added some code comments, but left the logic as is. Btw, just in case you want to take a stab at it and it crosses your mind, IMO, we should not use a computed here even if it initially looks like a good refactoring, because this needs to be bidirectional (read and write), and on write it needs to do some maths to update some other variables, which would be a side effect in a computed getter and that should be avoided :)