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

Only parse and display a limited number of rows #976

Closed
ellisonbg opened this issue Sep 27, 2016 · 5 comments
Closed

Only parse and display a limited number of rows #976

ellisonbg opened this issue Sep 27, 2016 · 5 comments
Labels
bug pkg:csvviewer status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Milestone

Comments

@ellisonbg
Copy link
Contributor

Right now the CSV widget blindly parses the entire file and tries to display it. The d3 parsing library we have can parse only certain lines of a file. We should only parse and display the first N lines of a CSV file, where N is a number we play with to see where performance problems hit. Guessing that 500-1000 is a good start...

@ellisonbg ellisonbg added this to the 0.90 milestone Sep 27, 2016
@ellisonbg
Copy link
Contributor Author

Pinging @sccolbert as this will be solved in a better way by the grid widget he is building...

@ellisonbg
Copy link
Contributor Author

While we are doing this, let's also put the table header and body in thead and tbody elements to pick up the full default table styling we are offering. Note: I am adding the needed CSS class to pick up that style in a separate PR...

@sccolbert
Copy link
Contributor

I can tackle this, or the thead/tbody portion of it at least.

For the partial parsing, is this really needed for the demo? If we do it, are we actually needing to implement a virtual renderer here? I assume you'd still want to be able to scroll through the entire data set?

@blink1073
Copy link
Contributor

blink1073 commented Sep 28, 2016

I checked the docs for d3, and it always parses the entire file. We can dynamically choose which of those rows to render based on scroll position, but IMO that should wait until we have a grid widget. I think the best we can do now is set a hard limit on the number of rows and console warn if it is above a certain number.

@ellisonbg
Copy link
Contributor Author

Fixed for now in master...

@lock lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Aug 10, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug pkg:csvviewer status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

No branches or pull requests

3 participants