refactor(drag-drop): add sorted event #13887
Conversation
googlebot
added
the
cla: yes
| /** 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.
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.
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.
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.
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.
Sure, but it'll be the odd one out since none of the other events have it.
crisbeto
force-pushed
the
13863/drag-sorted-event
branch
from
3f89c36 to
b848344
Compare
|
The feedback has been addressed. |
|
I see that you just added the If you want your PR to be merged, it has to pass all the CI checks. If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help. |
|
Just needs a rebase now |
|
Hi @crisbeto! This PR has merge conflicts due to recent upstream merges. |
crisbeto
force-pushed
the
13863/drag-sorted-event
branch
from
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
cdkDropListSortedevent that emits the item's previous index and current index as it's being sorted in a list.Fixes #13863.