Closed anthonydresser closed 2 years ago
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?
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.
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.
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.
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/
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?
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.
@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.
@ghiscoding thanks, I'll take a look at those changes.
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.
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.
@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!)
@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.
When dealing with large row counts and when updating the row count, slight rows shifts occur even when the user is not scrolling.
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 thiscj
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?