evidence-dev / evidence

Business intelligence as code: build fast, interactive data visualizations in pure SQL and markdown
https://evidence.dev
MIT License
3.94k stars 188 forks source link

[Bug]: DataTable (pagination) - current page number left stale when filter results in less pages than the current page #2084

Closed paulmech closed 1 week ago

paulmech commented 1 month ago

Describe the bug

Noticed an issue with DataTable that affects pagination, when filters that affect the rowset are changed, and produce a subset with less pages than the currently selected page.

This does not affect the search field. An example as seen in the screen shots attached, is having an external buttongroup or dropdown as a filter. evidence-datatable-1 evidence-datatable-2 evidence-datatable-3

Steps to Reproduce

  1. write a query that triggers pagination
  2. add a filtering widget (e.g. dropdown, buttongroup, etc.) that reduces the number of pages when activated. You must be able to toggle it on and off
  3. Start with the filter that produces the most amount of rows
  4. select the last page
  5. Toggle the filter to the lesser result size

Pagination should now show a current page that is greater than the total number of pages

Logs

No response

System Info

System:
    OS: Linux 5.15 Ubuntu 22.04.4 LTS 22.04.4 LTS (Jammy Jellyfish)
    CPU: (4) x64 Intel(R) Core(TM) i7-3667U CPU @ 2.00GHz
    Memory: 2.96 GB / 7.59 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 18.20.2 - ~/.nvm/versions/node/v18.20.2/bin/node
    npm: 10.5.0 - ~/.nvm/versions/node/v18.20.2/bin/npm
  Browsers:
    Brave Browser: 125.1.66.118
    Chrome: 112.0.5615.49
    Chromium: 125.0.6422.141
  npmPackages:
    @evidence-dev/core-components: ^4.3.0 => 4.3.0 
    @evidence-dev/csv: ^1.0.8 => 1.0.9 
    @evidence-dev/duckdb: ^1.0.9 => 1.0.9 
    @evidence-dev/evidence: ^36.0.0 => 36.0.0

Severity

annoyance

Additional Information, or Workarounds

Patched locally by appending

        if ( currentPage > pageCount ) {
            currentPage = 1
        }

after https://github.com/evidence-dev/evidence/blob/5ec75c1a4f180e1f1c7420e31a89f35eb2a4422b/packages/ui/core-components/src/lib/unsorted/viz/table/_DataTable.svelte#L334

archiewood commented 1 month ago

Would accept a PR to fix this

archiewood commented 3 weeks ago

this issue has now resurfaced

@jdimmerman fyi

archiewood commented 3 weeks ago

specifically when a search is performed and then cleared, the page index is not reset