-
Notifications
You must be signed in to change notification settings - Fork 6.8k
refactor(drag-drop): add sorted event #13887
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
/** Event emitted when the user swaps the position of two drag items. */ | ||
export interface CdkDragSort<T = any, I = T> { | ||
/** Index from which the item was sorted previously. */ | ||
previousIndex: number; |
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.
Will this be the index since the last time a sort event or since the beginning of the drag?
Should be provide the previousContainer
and currentContainer
?
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 since the last sort. I don't know how useful the previousContainer
will be in this case and it'll add some complexity since we don't have it easily available in _sortItem
.
This should be marked as |
I wasn't sure exactly how to mark it, because on one hand it's a new public API, but on the other it's just exposing some info that we had already. |
src/cdk/drag-drop/drop-list.ts
Outdated
@@ -101,6 +101,10 @@ export class CdkDropList<T = any> implements OnInit, OnDestroy { | |||
@Output('cdkDropListExited') | |||
exited: EventEmitter<CdkDragExit<T>> = new EventEmitter<CdkDragExit<T>>(); | |||
|
|||
/** Emits as the user is swapping items while dragging. */ |
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.
Could you say "actively dragging" just to make it more clear that it while the interaction is occurring?
src/cdk/drag-drop/drag-events.ts
Outdated
@@ -71,3 +71,15 @@ export interface CdkDragMove<T = any> { | |||
*/ | |||
delta: {x: -1 | 0 | 1, y: -1 | 0 | 1}; | |||
} | |||
|
|||
/** Event emitted when the user swaps the position of two drag items. */ | |||
export interface CdkDragSort<T = any, I = T> { |
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.
Can you call this CdkDragSortEvent? I forgot to mention this, but for v8 we want to make all of our emitted event objects consistently named with Event at the end.
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.
Sure, but it'll be the odd one out since none of the other events have it.
3f89c36
to
b848344
Compare
The 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
Just needs a rebase now |
Hi @crisbeto! This PR has merge conflicts due to recent upstream merges. |
b848344
to
ce567b8
Compare
Adds a new `cdkDropListSorted` event that emits the item's previous index and current index as it's being sorted in a list. Fixes angular#13863.
Adds a new `cdkDropListSorted` event that emits the item's previous index and current index as it's being sorted in a list. Fixes #13863.
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. |
Adds a new
cdkDropListSorted
event that emits the item's previous index and current index as it's being sorted in a list.Fixes #13863.