isaacHagoel / svelte-dnd-action

An action based drag and drop container for Svelte
MIT License
1.76k stars 105 forks source link

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

Open vini-brito opened 1 month ago

vini-brito commented 1 month ago

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 commented 1 month ago

Thanks. Most likely it's a svelte 5 bug and not a lib bug but I will block some time to have a look

On Fri, Aug 9, 2024, 15:39 Vinicius Brito @.***> wrote:

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 https://svelte-5-preview.vercel.app/#H4sIAAAAAAAAA-0aC1Mbx_mvfFESI1y9MMWND44MQYrNGAcqcFOP5WFWt99JO5x2r3t7gKzov3f2pXsgUeO2004CA0K3-71fe_d9t2jELMGsEXxcNDiZYSNoHKVpo9VQ81RfZDeYKGy0GpnIZaRXDrJIslQdjvhIsVkqpIKF_j5SlNPPgmPLXl28Oeqf_Xp1fnp0PHhzdtofDK9OLgfvrk76DuByePL69WB4UYU3MO-Ohm8Hw6vz4dn5YHj54eqXo3cDD3b2fng8uNAXS4ilmMGWlbFNOW2TSDHBt_bLwkGcsBSqwF3C2YwoNJBl2EgiUTi4Qa76LEuJiqYoa8geqYJ4K5ki4wTrjDIlpMEAAOh2oY8x4whqimC2MqBMYqSSOTAOasoyiMQsFRy5skh4Z1hEgmcKMkwwUkhPFM5OKIQrvk2eJ8n2_hoULihmx5pmhhWMj5-2rSZ8pCwodSpDuN4QTYfgwLVh-7kk2ubvMghhr-f2E1QwEzKd9lmmeWm-SuZY2o6QK4l9SSYTpGf8OJeZkBBCTJKsDEclmZSolLfjnBt_Q6aIVJpUk9EW4LaPSOykEo0KGJM8UVZ-HaobiKqRqlq4k6FqMmrxljW2U8Jpgm9x3he3vFmwZTE0m9i5xjmEYQhbA65QbsFvv0FpEba24dmzinrbG5X1rJ1jtV2Ywpm2-Ufr8oX9p39GjYgonAg5HzUCGDXGiYiuR41WBUJwpSPMAFzinQ2TGpAiEwuQ1jYYtesK79TV3vXP6mWkzpLT_Oru7-Pjo58-D0__ehfVkVIpUpSKYaaRS_LabS5mfTE7rwB9rALV9KzhkxlaqU7JGJMK8ypkRFIyZglTzj4DytQD4NTGzt9IkmNhr1HjPvxyDY0vkPfY--K_KLGuKywDAuo_K_s5kcjVCX2E8EMkD4HfF36tuNWlT616ONEviaU6leLS7Xir_O-y6x9_eZWf9nZ3CWV8Hr_7NY0n_dmOfP3qKbuesut3kF36vPy_ya7d3af8eqy8T_n1sPB_9Pwi5CnDvFueMuwLhH_KMPiqE2xv7ynDnjLsS4R_yjB4bIYRQsZPpxjAU479HnNsxD_t19qLuv9fX6OYKt0b7tU3MjQNUdOQ3pJCqAssGvGujzsl2fGUJVQir7Vxvwtqm7a1-eyZ_dJJkE_UFA6hV-_72gZsn9NjwTNGUZaasN0uHEURZpnptVNUhCXgsnMOIjbLpjls4W-I9FAhYMd-9Z3-GjUrIOEUGI8FFDnv6VrsgjDH2xPXr7VbHUNiv4AwhIpdHosNvO0gxjBXkk0mKNfw1_gFbYcSmuWOvSqx9mTcvrtcsfeNZq9DsRFD05MOQ3CTmc7bwYefzo6Gfe3AFekwBD_v6fSHR6-vLi7Pzs8H_ZW77vXk_bhA2eb3huZ7n9OfGScJ-4x_HN_XNfkybz_szHE-Hif4Pj2TF6guJeJq1rPR0ednJ79cDob_rgu1HMcJi67t8EZ7pRjg6KtOpkSqyxyZmFFTc3tfT9LO7WjHYtihm1GC8QnkKSgBqSnbgAnOkKusUOa7-hgtDIHRkiLdLpxY20ZaMqTG7eZsSSQSOl8N4jzGmrGRnchpUd9zu70KIGcUwCTDguuG0ZOmcIEW2YM4eSjkmdZ3NU58wNprPFweWLniHobQKxmiMjy0Eumw8XO0ugarQHAjw-ZWnlKijI-zre1aMBx0i1kyP3CHiJmq0nCxOlMMdJSQLAv1uZmq6cJ8LvWJ6XYCLvz5ES6-KZ0mBjnPMHDD6XCxsBncqk0vW-bEa1Wnlq0NU8qlISt45I6dcHHvJHIQsStOJQhfr5Yjboboi2-RRFN31sUsUSibxsrbEB6a5Q6j8I1OvI0j9W0gmQ2JpkPYXlpbH1B2473iht6B1lwbgjrtg_okd7n0KN7ujN-wjI0TbGt6jcp24IMyXKxNLCvQiqTgJqfCRdPlenh4vxJYHF8OHO6hJ7H4lsWWrn8SsDNVfddPGEe5tSxicaQO7CxeyxkXqyM1ZpwGxu7hwnLU35dlEB0U4aixKiGN8qYJw9BGI_wJdiqYq_j1tGllW_BSZoSLe8lZAu4e-kJcd6iRkGg18C5cVKrvj9CDANpVmYhkpJ3oBwedSpJM2tbwVbW8z9ftZWqe4D1WW5HJiwAmkoy3IKgujBmfbNWUn4k8QypueSUMSnP8tQHgsZXIo6kB_ir0a5zfY70og4xUbbRvwParIBu5VeCWJd6HB13Kbooo7rJ4Wbh2U1Cbx9uy_Q5SYKWoApuEUZ4pMWtnU0LFbVtvWpCP_-rFmk_Lw7LAlq57fi5z7ab3JDfLXqVFV9cxW9Zt7Lu6rkPGQFB24-08RTaZqgBiptqOlzPbLaNqum7Dh1QqmH6fwq2mImO2hklMiGI3lfclKLvprI5Mx3osJEUZwIv0DjKRMArjJK9gOfG_ArNTqZMeb6NKm62QEkoZnwTuWWukZkROGC-uvSxc8KoEphr1POsZ423PZafX-36_WHZS7e710jtPlETXEylyTtuRSLSx5WRMmjsvd1qws_uD--h1env6ruQ5HEGWj1WCUCCCQYTn3aqYOyuTGYovdl-2YHfX_vU6O55coiUFi1SicefCOoBeemfM_-f0zlLqtcD9FlQyS8birMiIlERMzQPodV4ZwCHSPEJw6xALCcQrhHGs79c8rpKE-zjz8L3ObgZIMmwz3ha5sswp1URmQqgpFFiOUDW-Ai5U0_lru54YJFeimhGllcf4aVPoWoxXvRa8NL_Gfo82-BrTPsJiFlTfLCZkru9C8G5dHhY3d3UrvSpi11lpteJodJ_DG3GD0ntUCZiRa30Xrm_1ELhQLELz5p7gYCqLfYnQucz7amqILGr67mywmLbWD-sstld5k6xzv2h7FqaMmH5SAK6k7N9PKUqyKVKYSJzfC43AZtOqRFXctGui9VTcoly5pxKjHXsCrt7t9D4y9UbjvmHUvsboAMdzcE2tVdYUtZmMM5HkygsirffaLwr3KZEGsLcqUCZ8YiFngc2ihCj80Gzv9b7frvq7RMIHxYuHClrJHOV7lDW2rVV452T22ZRlC9geC8_pc9vchQXwonb-uMjZZE9zuhuDXkzFbdmgt1Pk5to9xuoDhWVgyCEtvHXQ9Sdso9WYCcpihrQR6Ifv5aflPwEIG9m-4SsAAA==

— Reply to this email directly, view it on GitHub https://github.com/isaacHagoel/svelte-dnd-action/issues/596, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE4OZC3XHRLFFSWJKC4AUXLZQRI2TAVCNFSM6AAAAABMHZECH6VHI2DSMVQWIX3LMV43ASLTON2WKOZSGQ2TOMJUGQ3TMNI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

vini-brito commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

vini-brito commented 1 month ago

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 commented 3 weeks ago

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_ITEMID 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 commented 6 days ago

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.

isaacHagoel commented 6 days ago

Thanks for the update. Let me know if I can assist

On Sat, Sep 7, 2024, 06:21 Vinicius Brito @.***> wrote:

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.

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.

— Reply to this email directly, view it on GitHub https://github.com/isaacHagoel/svelte-dnd-action/issues/596#issuecomment-2334753893, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE4OZC4DT5RYULMMOEELHYLZVIFC3AVCNFSM6AAAAABMHZECH6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZUG42TGOBZGM . You are receiving this because you commented.Message ID: @.***>