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

Frozen Bottom Rows should render only the rows in the viewport #727

Closed
6pac opened this issue Feb 16, 2023 · 5 comments
Closed

Frozen Bottom Rows should render only the rows in the viewport #727

6pac opened this issue Feb 16, 2023 · 5 comments

Comments

@6pac
Copy link
Owner

6pac commented Feb 16, 2023

example-frozen-rows takes a long time to render when I open the page (maybe 5-6 seconds).

Debugging it led to

SlickGrid/slick.grid.js

Lines 4332 to 4338 in bb40f4d

// Render frozen rows
if (hasFrozenRows) {
if (options.frozenBottom) {
renderRows({
top: actualFrozenRow, bottom: getDataLength() - 1, leftPx: rendered.leftPx, rightPx: rendered.rightPx
});
}

frozenBottom is true and the frozen row is set to -5. A quick question @ghiscoding , does a negative row number mean 5 rows up from the end of the dataset? If not, I'm not sure how -5 was selected.

The example is then rendering everything from there to the end of the dataset, ie. all 50,000 rows are having a <div> created for them. Hence the long pause.
Surely the intent is to render just the frozen rows in the viewport. Or perhaps we are meant to render just the last 5 rows?

I'm happy to have a further look, I just suspect @ghiscoding this is an issue you will instantly know how to fix.

@ghiscoding
Copy link
Collaborator

ghiscoding commented Feb 16, 2023

From what I remember only "-1" is the only negative value that is allowed and equals to "undefined" or "unset" frozen. The frozenBottom has to be a boolean and is used as top/bottom frozen section as you can image. That's pretty much what I remember and I think -5 is probably a wrong value.

Just a question on the release, the last 2 releases didn't automatically create the GitHub Release, I had to create them myself by copying the changelog. That used to work automatically with the GITHUB_TOKEN in the .env file, could it be that the file got deleted or that you changed the token?

@6pac 6pac closed this as completed in 008ed88 Mar 13, 2023
@6pac
Copy link
Owner Author

6pac commented Mar 13, 2023

Fairly easy fix once I got the time to look at it! Should improve the performance of a number of unit tests as well.

Also have regenerated the .env file which had disappeared somehow. I did have a bad corruption of Windows last time I went away and had to recover, so that might have been it.

@ghiscoding
Copy link
Collaborator

@6pac just a small side note, I don't think commits with PATCH will be picked up to be added into the changelog, I think only the word fix will work. See Conventional Commits for more info. Cheers

@6pac
Copy link
Owner Author

6pac commented Mar 16, 2023

@ghiscoding my bad, I keep getting it wrong. have made some notes for myself.

@ghiscoding
Copy link
Collaborator

no problem, you'll get the hang of it 😉

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

No branches or pull requests

2 participants