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

Rows shifting during rowcount updates #297

Closed
anthonydresser opened this issue Dec 7, 2018 · 12 comments
Closed

Rows shifting during rowcount updates #297

anthonydresser opened this issue Dec 7, 2018 · 12 comments

Comments

@anthonydresser
Copy link

When dealing with large row counts and when updating the row count, slight rows shifts occur even when the user is not scrolling.
jumpingrows

repro: https://jsfiddle.net/r47kqpc9/16/

I've tracked it down to when the row top is calculated https://github.com/6pac/SlickGrid/blob/master/slick.grid.js#L1597 the offset value is slightly different after each row count update. That is being caused by this cj value slightly changing each time https://github.com/6pac/SlickGrid/blob/master/slick.grid.js#L2027 since it is calculated based on the exact row count (not some modulo of the value).

It seems this cj value has something to do with dealing with the maxsupportedcss work around.

Is there some work around to this? Or some way to reduce the visual impact this has?

@ghiscoding
Copy link
Collaborator

That is being caused by this cj value slightly changing each time

How slightly is it? can you provide some numbers. Have you tried doing maybe Math.ceil(offset) on it?

@anthonydresser
Copy link
Author

The change tends to be about 1000, in my case, but changes based on the number of rows. Here are some examples I have pulled, (sequential row update calls). (I'm dealing with very large row counts.
image
image
image

I haven't tried making any fixes yet since I'm not too familiar with this area of SlickGrid. Figured I would ask before I start tinkering.

@ghiscoding
Copy link
Collaborator

Oh that is quite large of a number, so the Math.ceil won't help at all, I don't know of that CSS workaround, this is probably better answered by @6pac

@anthonydresser I was looking at the Microsoft repo that you're using this grid with, is there a live demo of it with the grid? Just curious about it and I don't have any Azure account.

@anthonydresser
Copy link
Author

The Microsoft repo is an Electron application rather than web based, so it doesn't require a Azure account. Its an application for managing SQLSever, so we use SlickGrid as our results grid when performing queries. We don't have a "live example" per say, but you can certainly test it for free; if you are interested we have quick start guides for install the application and locally installing a sqlServer instance to connect to:

https://docs.microsoft.com/en-us/sql/azure-data-studio/quickstart-sql-server?view=sql-server-2017

mac specific guide:
https://blogs.msdn.microsoft.com/bobsql/2018/04/24/take-the-sql-server-mac-challenge/

@ghiscoding
Copy link
Collaborator

Oh that is why it looks like VSCode 😉
I'll give that a try possibly on Monday. we do use SQL Server (not Azure) at work, so it might be useful

I'm glad to see SlickGrid is used in some Microsoft projects/applications 😄
It's been my favorite grid for years, even though I used other grids (like UI-Grid), I always came back to it. So last year, I decided to create 2 wrappers on top of it Angular-Slickgrid and Aurelia-Slickgrid, I love this grid. Also, hopefully we can get the frozen/pinning functionality done in the coming month(s) too (ref #26)

@6pac
Do you have any idea on this issue?

@6pac
Copy link
Owner

6pac commented Dec 8, 2018

I've had a quick look, you've diagnosed the problem well - this really looks like a weakness in the core Slickgrid offset management code.
Presumably the one-pixel variation is caused by a slightly varying fractional number falling alternately on either side of a pixel-boundary cutoff number calculated by the browser render engine. As you suggest, one way to handle this would be to calculate on a modulo of the total height. Another might be to only change the top offset if it is more than a specified (small) amount more or less than the existing offset, which amounts to essentially the same thing.

Either way, it's going to require a few hours of digging and tracing (and it's the kind of thing I'd probably end up mapping out on paper too), for which I'm afraid I don't have time right now. You've clearly got the test harness all set up though, so if you're up for doing this, happy to apply the resulting patch.

I suspect this may end up being quite tricky though, as it's relating to sub-pixel browser calculations - there may be some challenges in keeping the offset in lockstep with that.

@ghiscoding
Copy link
Collaborator

@anthonydresser can you try the change made in PR #270 and #268, I think #270 is a shorter version of #268, so I would recommend you try #270 first. I think it's related to your problem and if you could try it locally, that would maybe help/fix that issue.

@anthonydresser
Copy link
Author

@ghiscoding thanks, I'll take a look at those changes.

@6pac
Copy link
Owner

6pac commented Jul 26, 2022

OK, I've had a long look at this one. Unfortunately, I don't think it's possible to fix. It appears that the offsets are being set correctly, and strangely it's the text that's moving, not the border lines.

I think it's actually that the CSS itself has trouble with the really large offsets.

image

I note if I retrieve the 'top' prop from the DOM, it comes back with exponential notation, suggesting that it's losing precision internally.

One last idea would be to try reducing the MaxCssHeigh with the new defaults. I've run out of time this morning, but will try this later today.

@6pac
Copy link
Owner

6pac commented Jul 27, 2022

@ghiscoding @anthonydresser pleased to note that knocking a few zeros off the new maxSupportedCssHeight option appears to fix the issue. So in the end it looks like CSS simply being unable to cope with that large an offset accurately (it nearly gets there!)

@6pac 6pac closed this as completed Jul 27, 2022
@ghiscoding
Copy link
Collaborator

ECMAScript 2020 now supports Big Int if that helps 🤣 so is it this commit that fixed it?

@6pac
Copy link
Owner

6pac commented Jul 27, 2022

@ghiscoding cool! yup that's the commit, but as far as I can see it's a css/browser issue, not anything to do with js or slickgrid. the 'top' values in the image above come back from the DOM like that - it's not an artefact of js. possible the new bigint might be extended into the inner workings of the render engine ... which would fix this, I guess.

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

3 participants