Allow add non-draggable element in dropzone container

[mskocik]

Change how dndzone children are being iterated to allow non-draggable element into drozone container AFTER item array. Fix #452.

This will be possible afterwards:

<section use:dndzone>
  {#each itemsas item(item.id)}
    <div animate:flip>
      <!-- draggable -->	     
    </div>
  {/each}
  <span data-dnd-exclude>NON DRAGGABLE</span>
</section>

As of this comment:

It sounds like a can of worms of edge-cases and complexity but I am open to ideas. i am happy to look at more detailed requirements or a PR if you can provide.

Originally posted by @isaacHagoel in #452 (comment)

All tests passed and also all example REPLs I tried continue to works.

---

There is similar PR (based on name) #335 for this topic, but I would say, my PR is easier to reason about 😊

[isaacHagoel]

This seems to only allow non draggables within the container after the last child.
I understood the feature request as non-draggables can be anywhere within the container and in fact had people who wanted the first item (e.g some kind of a title) or middle items to be non draggable. This is where you get into all of the edge cases I was referring to.

[mskocik]

Yes, only as I wrote in the description. The related issue mentions also "before" position (which I noticed after rereading now), but I was referring to the video of desired outcome.

When I was playing with, I did all variants (before, inside items, after). Inside items it was possible only with some custom property defined on items themselves, but it worked, even toggling it. I just didn't want to combine it.
Placing non-draggable before items was more problematic, because lib doesn't expect this and dragging and droping looked bad.

I can add ability to allow marking items as non draggable through property. But maybe that could go into another PR?

Personally I think it's better to have something than nothing

[isaacHagoel]

I guess what I am struggling to understand is what is it that you can do with this feature that you can't do without it. Why not place the non draggable element right before the dndzone and have both share the same parent container. With css it will look like that element belongs in the list. Same question applies to an element at the end or even in the middle (use 2 dndzones and place the static item in between). I would appreciate it if you could explain. If we do go forward with something like you suggest the implications on aria need to be considered as well.

…

On Mon, Feb 26, 2024, 16:46 Martin Skočík ***@***.***> wrote: Yes, only as I wrote in the description. The related issue mentions also "before" position (which I noticed after rereading now), but I was referring to the video of desired outcome. When I was playing with, I did all variants (before, inside items, after). Inside items it was possible only with some custom property defined on items themselves, but it worked, even toggling it. I just didn't want to combine it. Placing non-draggable before items was more problematic, because lib doesn't expect this and dragging and droping looked bad. I can add ability to allow marking items as non draggable through property. But maybe that could go into another PR? Personally I think it's better to have something than nothing — Reply to this email directly, view it on GitHub <#544 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE4OZCYOFQQS75IVXNDQ4E3YVQORXAVCNFSM6AAAAABDY3KLL2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRTGM3DENZWGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[mskocik]

This REPL shows what's impossible to do correctly with wrapping div.

IMHO enforcing some type of markup shouldn't be required, when simple solution exists. And it exists, we just need to take into account that node.children don't strictly represent items property.

Aria shouldn't be concern at all, because "init" loop skips those non-draggable items (I am speaking in general, although this PR is only about after non-draggables, which handles) so they are not participating in dnd logic in any way.
Sidenote: I am not sure aria works correctly, because aria related elements do not change their content when I focus or move some element by keyboard.

---

This PR could be extended to handle all cases (at least I can try to implement before, which was glitchy):

• before draggables
• inside items (this one would be possible only by some special property on item object)
• after - already in PR

[isaacHagoel]

Nice example!
Regarding: "Aria shouldn't be concern at all", have you tried using it with a screen reader?
In your example will the instructions explain to the user that there is a non draggable but still focusable and usable button inside the zone at the same level as the items?
Regarding the "If we can do something we should" sentiment - I am the one that will need to maintain the lib and potentially add other enhancements on top of any change that is introduced and I have to consider the long term implications of making a change such taking the items length as a source of truths instead of the number of children. Also, I want to deliver complete features and supporting only one case, in which the non draggable is at the end feels incomplete.
Again, it's not that I am against the idea. I think that a more complete implementation would use some data attribute to mark elements as non draggable and would account for different ways it might be used (including flipping it dynamically).

[mroforolhc]

Why not place the non draggable element right before the dndzone and have both share the same parent container. With css it will look like that element belongs in the list.

I thought the same solution, but I catch bug: when I set overflow: scroll on the parent container and add below exactly the same container, dndzone into the first container will be overlap dndzone into the second container.

REPL: https://svelte.dev/repl/77eb045ce8044b5aac30df935492cdfa (I try to add a button under list into the scrollable container)

firefox_jVm1NhYVNJ.mp4

[isaacHagoel]

thanks @mroforolhc !
I created #609

[isaacHagoel]

FYI @mroforolhc fix here: #610
I still need to make sure it doesn't break anything else before I merge and release but looks good so far