Skip to content
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

DataTable: Missing rows when filtering with VirtualScroller in Firefox #5118

Open
sasa0103 opened this issue Oct 19, 2023 · 21 comments · Fixed by #6472
Open

DataTable: Missing rows when filtering with VirtualScroller in Firefox #5118

sasa0103 opened this issue Oct 19, 2023 · 21 comments · Fixed by #6472
Labels
Type: Bug Issue contains a defect related to a specific component.

Comments

@sasa0103
Copy link

Describe the bug

When you scroll down some rows and use the filter, no result is shown. In Chrome it works, but not in Firefox. I used the example from the documentation (virtualscroller) and added some filters to reproduce the problem in my application.

Reproducer

https://codesandbox.io/s/primereact-demo-forked-kfn7nh?file=/src/App.js:745-754

PrimeReact version

10.0.5

React version

18.x

Language

TypeScript

Build / Runtime

Create React App (CRA)

Browser(s)

Firefox 118.0.2 (64-bit)

Steps to reproduce the behavior

  1. Open example with table and filters
  2. Copy "Vin" value from first row
  3. Scroll down to e.g. line 20
  4. Enter copied text to filter
  5. Entry is not shown

Expected behavior

Row with the copied entry should be shown.

@sasa0103 sasa0103 added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Oct 19, 2023
@melloware melloware added Type: Bug Issue contains a defect related to a specific component. and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Oct 19, 2023
@sasa0103
Copy link
Author

sasa0103 commented Dec 7, 2023

Any idea when this will be fixed?

@melloware
Copy link
Member

No idea unless you have a PrimeTek PRO account and can request this fix through your PRO support.

@kl-nevermore
Copy link
Contributor

In Firefox, when content transitions from a scrollable to a non-scrollable due to filtering, the calculation logic is not triggered.

There is a temporary workaround: manually invoke the scrollInView when the items changed

There hasn't been extensive testing, so it's unclear if this workaround may introduce new issues.

@heshamwhite
Copy link

heshamwhite commented Mar 6, 2024

There is a temporary workaround: manually invoke the scrollInView when the items changed

@kl-nevermore On which element should scrollInView be invoked?
Because I tried to call it on the p-datatable,p-datatable-table and p-datatable-tbody without success.
It couldn't be invoked on a "tr" as the tbody is completely empty hence "Missing rows"

Thank you :)

@kl-nevermore
Copy link
Contributor

const tableRef = useRef()
<DataTable
        ref={tableRef}
        value={cars}
        filters={filters}
        filterDisplay="row"
        globalFilterFields={["vin", "year", "brand", "color"]}
        scrollable
        scrollHeight="400px"
        virtualScrollerOptions={{ itemSize: 46 }}
        tableStyle={{ minWidth: "50rem" }}
        onValueChange={(e) => {
        // try it
          const vs = tableRef.current.getVirtualScroller();
          tableRef.scrollInView();
        }}
      >

@heshamwhite
Copy link

Thank you for your reply but neither the tableRef object not the tableRef.current has a scrollIntoView function

tableRef.scrollInView is not a function

TypeError: tableRef.current.scrollInView is not a function

am I missing something?

Here is a sandbox of the change.

@kl-nevermore
Copy link
Contributor

kl-nevermore commented Mar 7, 2024

ohh I pasted it wrong
@heshamwhite
https://codesandbox.io/p/sandbox/primereact-demo-forked-9r6dqh?file=%2Fsrc%2FApp.js%3A32%2C20

@kl-nevermore
Copy link
Contributor

20240307161630_rec_

@heshamwhite
Copy link

super, yes it works. Thank you for your help!

Hopefully the original bug would be fixed soon

@melloware melloware added the Resolution: Workaround Issue or pull request contains a workaround. It needs to be reviewed further by Core Team label Mar 7, 2024
@ssode
Copy link

ssode commented Apr 24, 2024

Hey, I recently came across the same issue and tried applying the workaround mentioned here, but noticed that when using TypeScript VirtualScroller.scrollInView expects at least 2 arguments. It seems to work for the filtering issue either way if casted, but now I'm seeing some weird behavior if I scroll down a bit and then sort on a column. The scroll bar will jump to a seemingly random position and not display any rows until the user manually scrolls the table again whereas in Chrome doing the same thing will reset the scroll position to the top and rows will be displayed normally.

https://stackblitz.com/edit/k45amg?file=src%2FApp.tsx

@melloware
Copy link
Member

I don't see scrollIntoView in your reproducer?

@ssode
Copy link

ssode commented Apr 24, 2024

There is no scrollIntoView method on VirtualScroller unless I'm missing something here? I call scrollInView on line 43, but as I mentioned I cannot even call it in the same manner that the workaround uses without typecasting it.

@melloware
Copy link
Member

what is this statement then? "VirtualScroller.scrollInView expects at least 2 arguments"

@melloware
Copy link
Member

OH I see the typescript needs to say props are optional on scrollInView

@ssode
Copy link

ssode commented Apr 24, 2024

Exactly yeah, are you also able to see the other issue I mentioned with sorting in Firefox?

@melloware
Copy link
Member

I didn't check that other virtual scroller in firefox issue but i believe you.

@melloware
Copy link
Member

@ssode interesting your using ScrollInView with NO params but it looks like index is required as to what index you want to scroll to. You left yours out so i wonder if JS is just making that value 0?

@melloware
Copy link
Member

@ssode PR submitted to fix the typescript but I didn't make Index optional i made the to value optional. Can you review?

@ssode
Copy link

ssode commented Apr 25, 2024

Hey @melloware, looks good. I think what was happening when calling scrollInView with no params was it would fall through to the last case where it calls scrollToIndex and in there there seems to be some logic that would default the index to 0.

@ssode
Copy link

ssode commented Apr 26, 2024

Hey @melloware , I'm not sure if this bug should've been closed. What I was trying to show in the reproducer I linked was that using the workaround mentioned here seems to cause an issue similar to the original when you add sorting into the mix, where after sorting a column the rows are invisible in Firefox, and also where the scrollbar ends up after sorting is not consistent in Firefox, whereas in Chromium-based browsers, the scrollbar will always reset to the top after sorting.

It seems like there is a larger issue at play when it comes to the VirtualScroller and Firefox that this workaround doesn't completely solve. Let me know if I should create a separate issue.

@melloware melloware reopened this Apr 26, 2024
@github-actions github-actions bot added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Apr 26, 2024
@melloware
Copy link
Member

yep it got closed when i merged my PR I reopened it as I only fixed the Typescript issue.

@melloware melloware removed the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Apr 26, 2024
@melloware melloware removed the Resolution: Workaround Issue or pull request contains a workaround. It needs to be reviewed further by Core Team label May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issue contains a defect related to a specific component.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants