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

Estimate table height #54

Open
j-f1 opened this issue Feb 4, 2021 · 2 comments
Open

Estimate table height #54

j-f1 opened this issue Feb 4, 2021 · 2 comments
Labels
enhancement New feature or request

Comments

@j-f1
Copy link

j-f1 commented Feb 4, 2021

Currently, the Table view renders only the first two screens’ worth of data, rendering more as you scroll. However, this can cause problems with inertial scrolling as well as making the scrollbar’s size misleading. One way to fix this would be to add a spacer <div> below the table with a height of (average table row height) * ((number of rows) - (number of rendered rows)).
Assuming table rows are of similar sizes (which should be correct in most cases) it will result in an accurate estimation of the overall scroll height, and it will only get more accurate as the user scrolls down. A downside to this approach is that if the user scrolls very quickly they may see a flash of white but since you render rows synchronously and fairly quickly this should not be that big fo a problem.

@j-f1
Copy link
Author

j-f1 commented Feb 4, 2021

I did a quick test by copying the source code into a notebook and this seems to work well (I’m happy to open a PR!)

// at the beginning of render(I, j):
setTimeout(() => {
  const averageRowHeight = tbody.clientHeight / (n - 1);
  const estimatedHeight =
    averageRowHeight * (data.length - tbody.children.length);
  spacer.style.height = length(estimatedHeight);
}, 25); // ideally do something else that triggers once the newly created rows get laid out by the browser

// when creating the DOM nodes:
const spacer = html`<div>`;
const root = html`<div class="__ns__ __ns__-table" style="[snip]">
  <table>[snip]</table>
  ${spacer}
</div>`;

@mbostock mbostock added the enhancement New feature or request label Feb 4, 2021
@mbostock
Copy link
Member

mbostock commented Feb 4, 2021

Good idea! Thanks for the suggestion.

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

Successfully merging a pull request may close this issue.

2 participants