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.

Sign up for GitHub

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

Jump to bottom

Not removing 'data-is-dnd-shadow-item-internal="true"' property so elements get hidden permanently #596

Open
vini-brito opened this issue Aug 9, 2024 · 13 comments
Open

Not removing 'data-is-dnd-shadow-item-internal="true"' property so elements get hidden permanently #596

vini-brito opened this issue Aug 9, 2024 · 13 comments
Labels
question Further information is requested

Comments

@vini-brito
Copy link

Hello! In Svelte 5 I am having an issue where once I release an element, it keeps the HTML property added by the library data-is-dnd-shadow-item-internal="true", which means the "visibility=hidden" CSS property is kept, which means the element is still there, still occupies space but cannot be seen, loses visualization.

In Svelte 4 it works as expected.

Using the migrate script doesn't help.

In the reproducible example below, just drag the elements around and they will lose their visualization but will still exist and can be inspected via regular context click -> Inspect to see that the HTML property is still there.

Reproducible in Svelte 5 preview here
@isaacHagoel
Copy link
Owner

isaacHagoel commented Aug 9, 2024 via email

@vini-brito
Copy link
Author

Thanks! If there is anything else I can do let me know, I can spend more time on it to help debug or even fix in case it is a low hanging fruit a non-maintainer could do. It is the only external dependency in my project, so it would be worth it for me to be acquained with its internals.

@isaacHagoel
Copy link
Owner

hi,
I had a quick look. If i remove the filtering of the shadow item on line 285 (#each items...) the problem seem to disappear. This example is quite complex and it is hard for me to understand what you are trying to do just from looking at the code. Can you provide some context?

@isaacHagoel
Copy link
Owner

also I pasted it to a normal REPL (not Svelte 5) and it is broken as well. I think the issue is in the code. If you explain what it is supposed to do I can try to help you fix it

@isaacHagoel isaacHagoel added the question Further information is requested label Aug 11, 2024
@vini-brito
Copy link
Author

Ah it looks a bit odd because I stripped some code down from my actual page contanining this, brought stores into it etc.
For context, it is a drag and drop webpage builder, from the viewpoint of the library it is a recursively nested element, using <self>.
I had some ideas and I will go through them sometime this week in order to debug and I will let you know.
I appreciate you looking into it!

@vini-brito
Copy link
Author

vini-brito commented Aug 19, 2024 •
edited
Loading

Ok, so I properly rewrote it to Svelte 5 and inspected things some more.
A difference that I found is that in Svelte 4, items are always objects.
Now, on Svelte 5, props are not regular objects but rather proxy objects:

image

I bet that interferes with the library functions, when it tries to target regular objects and finds these proxies.
Apart of that, I did not find anything else different from 4 to 5 in the data that is passed around.
Not saying that this proxy thing is the problem, just that it is the only difference I could see, of course it could be something else.

Here is the properly updated reproducible example.

And removing the filtering of the shadow items indeed solves that problem, but breaks the nested implementation, this is how it looks broken:

image

And how the Svelte 4 version looks, which is the functioning version:

image

The filter idea I got from the docs: "you might want to import SHADOW_PLACEHOLDER_ITEM_ID in order to filter the placeholder out when passing the items in to the nested component" just to give context as to why I wanted to have the filter too.

A Svelte 5 version showing how the container breaks is here.

I imagine it breaks because the library needs me to filter, which is why you made the recommendation to filter the placeholder out and even implemented the ID of the shadow element, so proceeding with the "no filter" path unfortunately is not a solution for a nested implementation.

So! After all, I can try to debug these proxy things that the new prop feature creates, would you have any pointers for where I can start looking inside the lib? Or maybe things to try in my code, I mean, external to the lib, using it as it already is.

P.S: Here is the working Svelte 4 version. (for some reason in the REPL it still breaks by having elements losing visibility when hovered even using Svelte 4, but locally this version with Svelte 4 always works, now I'm more confused than ever, maybe this bug was there all along and proxies just made them come forward? Maybe something related to the speed of the REPL vs localhost or some other race condition? So many questions).

And also its equivalent, with sources block, broken containers due to not filtering Svelte 5 version.

P.P.S: Yeah I don't think it is a Svelte 5 issue. I picked my code, verbatim, that I run with npm run dev and it runs great in the browser and has run for months already. In the REPL I get this issue even with the Svelte 4 complete example above.
So far it seems like Svelte 5 just made the issue evident.

@vini-brito
Copy link
Author

vini-brito commented Sep 6, 2024 •
edited
Loading

Just a heads up, I've been debugging this and I found out it has nothing to do with Svelte 5, I am debugging in Svelte 4.
Maybe I will change the issue name if I can.

So far, what I am understanding is that when the decorator is applied the correct target element is not in the list, so the correct index ends up hitting another element next to it.

Immediately after that another extra decoration happens and hits the correct element, which is the one being dragged.
However, later the undecorate action then hits only the correct target, leaving the incorrect one with still having the decorated style.
And I mean extra because when the issue does not happen the decoration only happens once, in other words, when it hits the correct target element it happens only once.

I will still dig further, find out why the correct target briefly disappears from the list, what removes it or more probable what applies the condition on it that will cause it to be filtered out.

@vini-brito vini-brito changed the title Svelte 5: Not removing 'data-is-dnd-shadow-item-internal="true"' property so elements get hidden permanently Not removing 'data-is-dnd-shadow-item-internal="true"' property so elements get hidden permanently Sep 6, 2024
@isaacHagoel
Copy link
Owner

isaacHagoel commented Sep 6, 2024 via email

@vini-brito vini-brito mentioned this issue Sep 13, 2024
Closed
@vini-brito
Copy link
Author

I removed the filter and made several changes in userland lately and apparently this issue is gone, so the PR is unnecessary. However I cant isolate what change solved it, else I'd share it.

Unsure if I should close this issue, while the filter issue is real, it is no longer an issue for me, so I'm leaving it to you Isaac whether this should be closed or if anything more should be done about this issue.

@dkmooers
Copy link

I ran into this bug in Svelte 4 when I had nested drag-and-drop (2 levels), where the state of the blocks was stored in a writable store (using Superforms > $formData):

$formData: {
  blocks: [
    {
      id: 'block-1',
      items: [
        { id: 'foo', productId: 'XYZ' },
        { id: 'bar', productId: 'ABC' }
      ]
    }
  ]
}

You can drag-and-drop blocks to reorder them, and within each block, you can drag-and-drop items to reorder them.

When I tried to drag-and-drop block.items inside the block, I was getting some buggy behavior where multiple shadow placeholder ID items would show up in the DOM during consider as I moved an item back and forth to different positions, and also if I only moved an item one slot to the right or left and then dropped it, the dropped item would still have the data-shadow-marker-internal attribute set (can't remember what it is exactly), and consequently the element would be invisible (visibility=hidden).

I fixed these bugs by just maintaining blocks as a local variable to the component, and using:

$: if (blocks) {
  $formData.blocks = blocks
}

to reactively sync the blocks state to $formData.

Not sure how to trace the origin of this bug, but the fix works for me!

@jdkdev
Copy link

jdkdev commented Nov 12, 2024 •
edited
Loading

Experiencing this as well. Currently on Svelte 4.

@isaacHagoel
Copy link
Owner

isaacHagoel commented Nov 12, 2024 via email

@jdkdev
Copy link

jdkdev commented Nov 12, 2024

thanks @isaacHagoel !

So, originally my items were a computed list:

$: dropdownItems = realList.filter(mySelectedFilters)

I worked around it by making dropdownItems static. So first question, should it be able to work with a computed value?

My thought was that maybe because the elements are getting redrawn it's losing reference to the element that gets the data-is-dnd-shadow-item-internal=true and can't unset it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
question Further information is requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes wrongful element decoration
4 participants
@dkmooers @vini-brito @jdkdev @isaacHagoel