Fix: Really replace jQuery append with native append for renderRows

[RasmusBergHogia]

Unfortunately, I must have made a mistake when I tested my previous PR. For scripts run automatically still when they get appended. This is because the elements that are appended to are a jQuery HtmlElement, and therefore they don't use native append but jQuery append method.

So the new PR take out the html element first, and then make a append on it instead.

[ghiscoding]

can you remove the package-lock.json file from your PR? Because that might break the GitHub Actions CI since your push seems to empty that file

[RasmusBergHogia]

I can remove the update on that file, but what I can see it doesn't empty it out.

[ghiscoding]

open it and you'll see, it's pretty much empty, there's only 3 lines left

[RasmusBergHogia]

open it and you'll see, it's pretty much empty, there's only 3 lines left

Yeah, look lite strange in the comparing so I checked the file instead and look fine.

[ghiscoding]

if you're running Yarn instead of npm, it might wipe it out, not sure if that is what happened

[RasmusBergHogia]

if you're running Yarn instead of npm, it might wipe it out, not sure if that is what happened

Run npm install before cypress test only, I don't really know what happend with the comparing. Locally and when I check on the commited file so look it right.

But probably no need of update that file anyway...

[ghiscoding]

@RasmusBergHogia
I should have tried on my end (I did not before today with the new SlickGrid version) because it breaks on my side, it breaks in our Salesforce environment and also on some of the Slickgrid-Universal Cypress tests (I assume most probably Aurelia-Slickgrid as well). Have you ran any Cypress tests when changing this code (apart from SlickGrid tests)? Because at this point, we might have to rollback on this...

The errors I get in our Salesforce environment is $canvasTopL[0].append is not a function, we might need better error handling for this. Always assuming the jQuery element has at least 1 child might not be correct, it probably fails in my case because it's not filled!?

EDIT

I think it's Salesforce that doesn't like .append() which I have noticed in the past and I don't really know why but he's ok with .appendChild()
So I came up with code that works in our environment and is much safer, I use appendChild instead of append as this seems to be the real problem on my side (Salesforce is always more picky) and I also make sure there is really a native element before trying to use it (not quite, after running the Cypress tests on my side, I see it's still broken)

- $canvasTopL.append(x.firstChild);
+ $canvasTopL.length > 0 ? $canvasTopL[0].appendChild(x.firstChild) : $canvasTopL.append(x.firstChild);

hmm that doesn't always work since append works with string/node while appendChild only works with a node element and it's not always the case... this is a bit of a pickle. I think that a rollback is our best course of option at this point, I'll do a PR later today and add more Cypress tests to cover the defects in a frozen grid (there's probably no Cypress tests for frozen grids in this fork but I do have some in my own libs which is where I caught the issue)
even with append it's broken, it shows the following when using frozen feature, which is totally broken

EDIT 2
Actually the issue shown below is caused by another commit while adding the missing code for the Post Async cleanup, the issue can be seen in this Example, the error thrown is

Uncaught TypeError: Cannot read properties of null (reading 'removeChild')

which seems to be caused by this.parentElement being null in some cases. I'll look at fixing both issues later today

[RasmusBergHogia]

@ghiscoding I run cypress test for SlickGrid only, has not SlickGrid Universal and Aurelia SlickGrid project setup locally.
Thought it was safe change, sorry about the mess I have made. Hope you find a solution, unfortunately over my knowledge about SlickGrid and Salesforce. And I understand if you need rollback...

[ghiscoding]

@RasmusBergHogia it's ok, there's actually 2 issues in that same Example so it's a 2 in 1 😉