Muuri: Webcomponents: Event target in Shadow DOM

[osamamaruf]

Event target is always shown as the host element in dragStartPredicate. Hence custom logic like this fails;

const grid = new Muuri(this.$.grid, {
            dragEnabled: true,            
            dragStartPredicate: (item, event) => {
              // Prevent first item from being dragged. 
              if (grid.getItems().indexOf(item) === 0) {
                return false;                
              }
              // Here EVENT.TARGET is always the host element!             
              if(!event.target.classList.contains('handle')) {
                return false;
              }
              // For other items use the default drag start predicate.
              return Muuri.ItemDrag.defaultStartPredicate(item, event);
            }
          });

Possible solution can be instead of returning the target here (https://github.com/haltu/muuri/blob/master/src/Dragger/Dragger.js#L302)
we should check for the event path (composedPath | path | target).

I am happy to submit a PR for this issue.

[niklasramo]

Thanks for the report @osamamaruf ! This kind of stuff has not occured to me and it's a valid point you are raising.

At the moment you can access the source event via e.srcEvent and use that to find the shadow dom target. Isn't it supposed to be e.srcEvent.path[0] or e.srcEvent.composedPath()[0]?

In any case might make sense indeed to fetch that target automatically and add it to the dragger event object, but then again, is it wise to bloat that object more than necessary? Dunno, have to think about that.

[osamamaruf]

@niklasramo Thanks for the quick response. Using the srcEvent's target did not occur to me. But IMO the target that the create event is making should be corrected as its being passed as the target by Muuri. If this is done the srcEvent can be left as it is.
Also yes it can be done without making the change in the lib; where the code would look like this;

const grid = new Muuri(this.$.grid, {
            dragEnabled: true,            
            dragStartPredicate: (item, event) => {
              // Prevent first item from being dragged. 
              if (grid.getItems().indexOf(item) === 0) {
                return false;                
              }
              //But this would have to be done whenever the lib is using in WCs           
              const srcEventTarget = (e.srcEvent.composedPath && e.srcEvent.composedPath()[0]) || e.srcEvent.path[0] || e.srcEvent.target;
              if(!srcEventTarget.classList.contains('handle')) {
                return false;
              }
              // For other items use the default drag start predicate.
              return Muuri.ItemDrag.defaultStartPredicate(item, event);
            }
          });

The issue here would be that inside webcomponents this would have to be done whenever the target has to be accessed so my suggestion was to make it more WC friendly. For a solution this is what I had in mind (https://github.com/osamamaruf/muuri/commit/4cc955f7db23eb1e61435845ec60e8c1324ec3ad)

[niklasramo]

After thinking a bit more about this and reading this helpful article about shadow DOM events in general I would argue that it's better to keep the target property of the Dragger event identical to that of the source event target. For starters, at least you won't be in for a surprise as it's the same target as the srcEvent's target _and_ there is a good reason why browser retargets the events bubbling up from shadow dom.

E.g. if I build a grid in the normal DOM and I use web components _within_ the item elements, I would expect the event targets to be the _host_ elements and not the shadow dom internals. And that's how stuff if working at the moment.

So _for now_ I'd say that this is the best way to handle it:

const target = e.srcEvent.composedPath
  ? e.srcEvent.composedPath()[0]
  : e.srcEvent.path
  ? e.srcEvent.path[0]
  : e.srcEvent.target;

Maybe we could introduce a shadowTarget prop later on, but I'm a bit on the fence with that one too because originally I didn't even intend to create a special Dragger event and was thinking of just propagating the source event as is. I don't like the fact that we are currently creating a new object on every emitted event because it increases GC activity which in turn blocks the main thread. And also it would be an addition to the public API interface which would need to be maintained. But, I do see it being helpful too, not just sure if it's worth adding to the library as the shadow target can be fetched fairly trivially via the srcEvent.

[niklasramo]

Closing this one as the issue seems resolved. Let's keep the shadowTarget prop in mind for future.