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

Fixed issue #67: Limitation of row count due to the max height of any HTML Dom element #68

Merged
merged 2 commits into from
Oct 24, 2016

Conversation

ljin-dev
Copy link
Contributor

Modified the way of visible rows are rendered to avoid issues caused by the the combined height of all rows exceeding the limit in browsers.

Description

When loading a large number of rows to the fixed-data-table-2 component, rows at the bottom of the table may not be rendered currently. Sometimes it can even crash the browser. This is caused by the height limit of a HTML Dom element in the browser. Different browsers may have different limit. In the implementation of the fixed-data-table-2, it uses a sliding canvas container to expose rows that are visible depending on the window size and the scrolling position. The height of the canvas is the combined height of all rows, visible and hidden. If the combined height exceeds the allowed height of any element in the Dom structure, rows at the bottom with a top offset larger than the limited value, won't be displayed correctly.

Instead of using a sliding canvas and setting the height of the canvas to be literally the combined height of all rows, we can simply slide the visible rows up or down to by a calculated offset to bring them to the right position that they can be seen. The offset will always be less than combined the height of visible rows, which will never exceed the limit of any browsers.

Motivation and Context

It prevents loading a large number of data to the fixed-data-table-2 component. Please see issue #67.

How Has This Been Tested?

Manually tested in the following browsers:

  • Chrome in Windows 10 and Mac 10.12
  • Firefox in Windows 10 and Mac 10.12
  • Safari in Mac 10.12
  • IE11 and Edge in Windows 10

Screenshots (if appropriate):

Rendering 10 million rows with no issues.
loading 10,000,000 rows with now issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.

@KamranAsif KamranAsif self-assigned this Oct 19, 2016
@KamranAsif
Copy link
Contributor

Thanks for getting this in. I'm swamped with work right now but hopefully I'll get to this by end of the week

@KamranAsif
Copy link
Contributor

Cleaned up some of the unused styling. LGTM.
Will merge after travis build

@KamranAsif KamranAsif merged commit e5f13cc into schrodinger:master Oct 24, 2016
@ljin-dev ljin-dev deleted the RemoveRowCountLimit branch October 24, 2016 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants