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

Problem with removing ports #220

Closed
314159265meow opened this issue Aug 12, 2022 · 4 comments
Closed

Problem with removing ports #220

314159265meow opened this issue Aug 12, 2022 · 4 comments
Labels
answered bug Something isn't working fixed/done

Comments

@314159265meow
Copy link

Hi,

when you remove ports from a node, the other ports often get the wrong position and sometimes in our case have their Initialized property set to false. The results are not deterministic though, I think it a timing problem with the MutationObserver.

You can test it in your demo.
Browser: Chrome 104
Animation6_63

@zHaytam
Copy link
Collaborator

zHaytam commented Aug 13, 2022

Hello,

I can confirm the bug. I'm looking into it, thank you for letting me know.
It does look like the link is as if it's going from another port (right or top/right)

@zHaytam
Copy link
Collaborator

zHaytam commented Aug 13, 2022

Got it!

The issue is due to how Blazor detects changes inside a for loop.

Let's take the default node widget:

    @foreach (var port in Node.Ports)
    {
        <PortRenderer Port="port" Class="default"></PortRenderer>
    }

It happens that the deleted ports are:
image

Which leaves the Right port right? Yes! But because of the for loop & blazor, the Top port is still rendered instead.
To fix it, we simply need to use the @key property:

    @foreach (var port in Node.Ports)
    {
        <PortRenderer @key="port" Port="port" Class="default"></PortRenderer>
    }

@zHaytam
Copy link
Collaborator

zHaytam commented Aug 13, 2022

This also explains why it looks like in the demo that removing ports removes them anti clock wise. The thing is, it's random, so it shouldn't even look like that 😄

@zHaytam zHaytam added bug Something isn't working need-user-input answered and removed need-user-input labels Aug 13, 2022
@314159265meow
Copy link
Author

Thank you quick help, it did some very weird things for us, like rendering non attached links and eventually throw a JS exception ("getBoundingClientRect" of null).

image

The @key attribute fixes it perfectly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answered bug Something isn't working fixed/done
Projects
None yet
Development

No branches or pull requests

2 participants