open-source-labs / Svelvet

🎛 A Svelte library for building dynamic, infinitely customizable node-based user interfaces and flowcharts
https://svelvet.io
2.59k stars 166 forks source link

Event Propagation for 3rd party library components #343

Open microdou opened 1 year ago

microdou commented 1 year ago

Thanks for the awesome Svelvet library! I will be event better if any 3rd party library component can work flawlessly in a Svelvet Node.

Currently there may be some issue in event propagation when incorporating a 3rd party component into a node. I'm testing the incorporation of carbon-components-svelte into svelvet nodes.

briangregoryholmes commented 1 year ago

Hey, microdou. So, this is kind of a tricky thing to solve because of Carbon's particular implementation. I'll start off by saying that our Node dragging logic does work with native HTML range elements if you need a quick workaround. In our click event listener, we ignore clicks on input, select, text area and button elements. This is a broad rule that works most of the time, including native sliders.

The primary issue here is that the slider from Carbon is really just made of divs. There does appear to be a hidden input in the tree of their component, but it isn't the thing you're actually interacting with. That's completely fine, but it makes it difficult for us to implement the logic around when interaction with these kinds of elements should or should not move the Node itself. It's not as simple as just ignoring everything that isn't the wrapping Node element, but perhaps we could make that a configurable prop.

I think table components are a great example. Intuitively, you would assume that you should be able to grab a Node on a non-interactive table cell and still be able to drag the Node around. But, because of the way Carbon has implemented their Slider, any general rule that covers both situations is going to be tailored a little bit to Carbon's particular implementation. Even though one is an input and one isn't, they're still both divs on the DOM.

The thing that comes to mind as perhaps the best option moving forward is querying the element to see if it has a tabindex attribute set to zero. This is true of Carbon's slider and I assume other component libraries have done something similar. We might run into a few situation where that is set incorrectly, but as long as we're transparent about it in the documentation, I don't see it being an issue.

Really, there just needs to be a consensus on what the defaults should be/are and documentation on how to work around these edge cases.

microdou commented 1 year ago

@briangregoryholmes Thanks for looking into this and the detailed explanation! I raised this issue in case it might be an edge case for library improvement. Totally agree that it can be mitigated by a better documentation in future :)

briangregoryholmes commented 1 year ago

We'll do both! I'll take a look around at the component library landscape for these kinds of things and see if we can make something a little more general. We'll then throw a tip callout in the documentation. Thanks so much for using the library and bringing this to our attention.