feat(cdk/drag-drop): support configurable propagation of mousedown an… #19334
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
googlebot
added
the
cla: no
|
@googlebot I signed it! |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
googlebot
added
cla: yes
…d touchstart events. Allows exposing native mousedown and touchstart events to parent elements so that listeners farther up the chain can respond before a CdkDragEvent has started. Without these events, certain gestures (swiper, etc) cannot be achieved when events are started on a DragRef element. Fixes: angular#19333
lincolnthree
force-pushed
the
event-propagation
branch
from
d5d5fc9 to
e7bf944
Compare
|
Fixed build error. |
There was a problem hiding this comment.
I think that this make sense for your particular use case, but I'm not sure whether it fits as a public API, because people would have to be familiar with the internals of the drag-drop module to know what the implications would be.
Another issue is that this only disables the stopPropagation on mousedown/touchstart events. I think that people may assume that if propagateEvents is set to false, then we wouldn't propagate any events.
You should be able to resolve your issue by binding your events with event capturing turned on.
|
Hey, @crisbeto Thanks for reviewing. In this case, we don't have control over the event listener bindings (and to be most configurable, I'd think the option would help people integrate with additional frameworks more easily.) To which other events are you referring? Would it make sense if the property were named something more specific, like, "propagateNativeDomEvents"? |
|
@crisbeto Hi :) |
|
Sorry for the delayed response, I must've missed the notification. I'm still not sold on the idea of this being a public API since it assumes that people would know the internals of the |
|
My 2 cents, summarized, and sorry if this sounds at all like a lecture or telling you what to do, I'm already working on a fork and this not being included will not really affect me -- just trying to be helpful and give a "real world" perspective that I think will help people: Drag & Drop is part of the angular "component development kit", and people need control over how it behaves when building custom components. Without fine-grained control, ability to use features will be extremely limited to "general purpose" use-cases that you guys think up but that may not exactly fit real-world scenarios, forcing workarounds and hacks. I don't feel like events being fired is really an "internal concern" as people rely on events being fired for all kinds of reasons. Events are a fact of life in JS I think that forcing an event model and consuming events without a way to override is taking control out of the users hands in a way that can be problematic. This is the kind of fine-control needed to really build custom components for highly customized apps, which I assume is your goal :) Without it I'll just continue to fork. But it's here if you want it. PS. I have other config features along these lines that I would submit if you folks are interested. Ability to disable the placeholder element, for instance. Support for centering the preview elements on the fired event coordinates. Etc. |
|
Looking at the code, it seems like calling My take it that it would be better to solve the nested dragging issue another way, such that the component doesn't have to call |
|
Fine by me. I don't actually believe this call to |
|
Hello @crisbeto @jelbourn! What's the status on this issue? Is it waiting for corrections, or it will not be merged altogether? I currently have a use case that manifests this issue where this public API would be useful. I agree that this would assume people know the internals of the drag-drop module, but stopping event propagation is actually quite dangerous because it breaks the event listening flow of apps and even third-party libraries. And worse, we can't actually fix it without removing whatever code is calling stopPropagation(). In the StackBlitz I explain how I'm not able to capture the click made inside a drag item in any way. While dangerous, the stopPropagation() in this case is useful to avoid breaking nested scenarios. But if the drag-drop consumer does not use nesting, then he should be able to deactivate its usage. So this option would be useful if a refactoring is not planned in the near-future. |
Some time ago we added logic to stop event propagation so that nested drag items don't trigger multiple sequences. Stopping propagation for all events seems to interfere multiple other use cases so these changes add some logic so that we only stop propagation when items are nested. There was something similar in angular#19334, but I decided not to move forward with it, because it required consumers to know the internals of the `drag-drop` module, whereas this approach can do it automatically. Fixes angular#19333.
Some time ago we added logic to stop event propagation so that nested drag items don't trigger multiple sequences. Stopping propagation for all events seems to interfere multiple other use cases so these changes add some logic so that we only stop propagation when items are nested. There was something similar in #19334, but I decided not to move forward with it, because it required consumers to know the internals of the `drag-drop` module, whereas this approach can do it automatically. Fixes #19333.
Some time ago we added logic to stop event propagation so that nested drag items don't trigger multiple sequences. Stopping propagation for all events seems to interfere multiple other use cases so these changes add some logic so that we only stop propagation when items are nested. There was something similar in #19334, but I decided not to move forward with it, because it required consumers to know the internals of the `drag-drop` module, whereas this approach can do it automatically. Fixes #19333. (cherry picked from commit e42d502)
|
The underlying issue should've been resolved in #21227 so I'll close this PR. |
|
Very good. Thanks for finding a solution. Getting closer to a usable basis for custom D&D needs. |
…ar#21227) Some time ago we added logic to stop event propagation so that nested drag items don't trigger multiple sequences. Stopping propagation for all events seems to interfere multiple other use cases so these changes add some logic so that we only stop propagation when items are nested. There was something similar in angular#19334, but I decided not to move forward with it, because it required consumers to know the internals of the `drag-drop` module, whereas this approach can do it automatically. Fixes angular#19333.
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes: #19333