-
Notifications
You must be signed in to change notification settings - Fork 6.8k
feat(drag-drop): allow connecting containers via string ids, attaching data to drop instances and consolidate global event listeners #12315
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Hi @crisbeto! This PR has merge conflicts due to recent upstream merges. |
5e95071
to
2163dbb
Compare
|
||
/** Keeps track of the event listeners that we've bound to the `document`. */ | ||
private _globalListeners = | ||
new Map<string, {handler: (event: TouchEvent | MouseEvent) => void, options?: any}>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alias (event: TouchEvent | MouseEvent) => void
into something like PointerEventHandler
?
const upEvent = isTouchEvent ? 'touchend' : 'mouseup'; | ||
|
||
// We explicitly bind __active__ listeners here, because newer browsers | ||
// will default to passive ones for `mousemove` and `touchmove` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need active handlers in the first place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's because we use preventDefault
to stop the user from scrolling the page or selecting text while they're dragging. If an event is passive, the browser will ignore the preventDefault
calls.
@Optional() private _dir: Directionality) { | ||
this._document = document; | ||
_registry.register(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about _dragDropRegistry
?
@@ -36,40 +42,58 @@ import {CDK_DROP_CONTAINER} from './drop-container'; | |||
], | |||
host: { | |||
'class': 'cdk-drop', | |||
'[attr.id]': 'id', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just [id]
?
2163dbb
to
87a430d
Compare
Feedback has been addressed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…g data to drop instances and consolidate global event listeners * Adds the ability to pass in strings to `connectedTo` when connecting multiple containers together. Previously we only accepted `CdkDrop` instances which can be inconvenient to pass while inside an `ngFor`. * Adds the ability to attach extra data to a `CdkDrag`, similarly to `CdkDrop`. This is mostly for consistency and convenience. * Introduces the `CdkDragDropRegistry` which is used to keep track of the active `CdkDrag` and `CdkDrop` instances. It also manages all of the events on the `document` in order to ensure that we only have one of each event at a given time. This will allow us to handle dragging multiple items at the same time, eventually.
87a430d
to
4af4c70
Compare
…g data to drop instances and consolidate global event listeners (#12315) * Adds the ability to pass in strings to `connectedTo` when connecting multiple containers together. Previously we only accepted `CdkDrop` instances which can be inconvenient to pass while inside an `ngFor`. * Adds the ability to attach extra data to a `CdkDrag`, similarly to `CdkDrop`. This is mostly for consistency and convenience. * Introduces the `CdkDragDropRegistry` which is used to keep track of the active `CdkDrag` and `CdkDrop` instances. It also manages all of the events on the `document` in order to ensure that we only have one of each event at a given time. This will allow us to handle dragging multiple items at the same time, eventually.
…g data to drop instances and consolidate global event listeners (#12315) * Adds the ability to pass in strings to `connectedTo` when connecting multiple containers together. Previously we only accepted `CdkDrop` instances which can be inconvenient to pass while inside an `ngFor`. * Adds the ability to attach extra data to a `CdkDrag`, similarly to `CdkDrop`. This is mostly for consistency and convenience. * Introduces the `CdkDragDropRegistry` which is used to keep track of the active `CdkDrag` and `CdkDrop` instances. It also manages all of the events on the `document` in order to ensure that we only have one of each event at a given time. This will allow us to handle dragging multiple items at the same time, eventually.
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. |
connectedTo
when connecting multiple containers together. Previously we only acceptedCdkDrop
instances which can be inconvenient to pass while inside anngFor
.CdkDrag
, similarly toCdkDrop
. This is mostly for consistency and convenience.CdkDragDropRegistry
which is used to keep track of the activeCdkDrag
andCdkDrop
instances. It also manages all of the events on thedocument
in order to ensure that we only have one of each event at a given time. This will allow us to handle dragging multiple items at the same time, eventually.