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

we should render 1/2 table of items before and after the on-screen rows #25

Closed
devoncarew opened this issue Sep 21, 2018 · 4 comments
Closed
Labels
enhancement New feature or request table component

Comments

@devoncarew
Copy link
Member

In order to make the virtual table drawing a bit smoother, we should render perhaps 1/2 - 1 full table's worth of contents before and after the visible rows.

@devoncarew devoncarew added the enhancement New feature or request label Sep 21, 2018
@DanTup
Copy link
Contributor

DanTup commented Oct 2, 2018

We used to do this, but when I changed it reuse the rows I removed it since I figured:

  1. We're populating the data on the next frame (via RAF) and inserted directly into the visible space, so I figured it was "always visible and up to date" (on my MacBook I can't repro any scrolling issues, or any white space)
  2. Rendering another screen above below essentially means 3x as many rows to re-render on every update (at least the way it works now - potentially we could make it re-order them moving from the start to the end, but that's not what it does currently) which seemed like it might make scrolling worse not better

Can you cause issues/stutters/white space when you scroll? Anything specific you're doing (how big is the log, how are you scrolling, etc.?)

@DanTup DanTup self-assigned this Oct 2, 2018
@devoncarew
Copy link
Member Author

I think there's always going to be a frame or two of latency - unrendered, white rows - as we're populating via javascript, and scrolling dom is likely highly optimized in the browser.

I do see what looks like some slight tearing at the top and bottom when I scroll quickly. Some over-rendering above and below would likely take the edge off that? If not a full page above and below, 1/2 page?

Rendering another screen above below essentially means 3x as many rows to re-render on every update (at least the way it works now - potentially we could make it re-order them moving from the start to the end, but that's not what it does currently) which seemed like it might make scrolling worse not better

I think if we did increase the over-rendering, we'd want to try and re-cycle rows that had already been rendered - i.e., just remove a few rows off the top of the table and add a few on the bottom, and not touch ones that were already present.

The tearing on scroll really isn't bad, but we may want to revisit this later to get slightly smoother scrolling.

@jacob314
Copy link
Contributor

I noticed the whitespace today playing around with the tool. Good news is there is an easy fix. The scroll event is sync but using RAF causes us to always miss a frame. There are some clever chrome apis if we cared about 60fps but for us I think all that matters is to general avoid flickering and hit at least 20FPS. Note there were a couple years where scroll was async in chrome which was really annoying for framework authors trying to infinite scroll.
https://blog.chromium.org/2016/05/new-apis-to-help-developers-improve.html

@kenzieschmoll
Copy link
Member

The table scrolling looks pretty smooth with the latest DevTools. I'm going to close this issue, as it appears fixed. We can re-open if this is still an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request table component
Projects
None yet
Development

No branches or pull requests

4 participants