Skip to content

Commit 9392318

Browse files
cdvv7788kesselb
authored andcommitted
fix: Copilot suggestions
Signed-off-by: Cristian <cristianvargasvalencia@gmail.com>
1 parent 6a1851c commit 9392318

File tree

3 files changed

+67
-25
lines changed

3 files changed

+67
-25
lines changed

src/directives/drag-and-drop/droppable-mailbox/droppable-mailbox.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,6 @@ export class DroppableMailbox {
2828
this.setStatus('enabled')
2929
}
3030

31-
update(el, instance) {
32-
this.options = instance.options
33-
}
34-
3531
registerListeners(el) {
3632
dragEventBus.on('drag-start', this._onDragStart)
3733
dragEventBus.on('drag-end', this._onDragEnd)

src/directives/drag-and-drop/droppable-mailbox/index.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ function onUpdate(el, binding) {
1515
const instance = instances.find((instance) => instance.el === el)
1616
if (instance) {
1717
instance.options = binding.value
18-
instance.update(el, instance)
1918
}
2019
}
2120

src/tests/unit/directives/droppable-mailbox.spec.js

Lines changed: 67 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
/**
2-
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
2+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
33
* SPDX-License-Identifier: AGPL-3.0-or-later
44
*/
55

66
import { DroppableMailboxDirective } from '../../../directives/drag-and-drop/droppable-mailbox/index.js'
7-
import { DroppableMailbox } from '../../../directives/drag-and-drop/droppable-mailbox/droppable-mailbox.js'
7+
import dragEventBus from '../../../directives/drag-and-drop/util/dragEventBus.js'
88

99
/**
1010
* Creates a mock DOM element with a firstChild for the directive.
@@ -42,6 +42,13 @@ describe('DroppableMailboxDirective', () => {
4242
// Use the Vue 2 hooks since the project is on Vue 2.7
4343
const { bind, componentUpdated, unbind } = DroppableMailboxDirective
4444

45+
let boundEls = []
46+
47+
afterEach(() => {
48+
boundEls.forEach((el) => unbind(el))
49+
boundEls = []
50+
})
51+
4552
it('tracks multiple instances independently', () => {
4653
const el1 = createMockEl('el1')
4754
const el2 = createMockEl('el2')
@@ -50,29 +57,57 @@ describe('DroppableMailboxDirective', () => {
5057
bind(el1, createBinding({ mailboxId: 1 }))
5158
bind(el2, createBinding({ mailboxId: 2 }))
5259
bind(el3, createBinding({ mailboxId: 3 }))
53-
54-
// All elements should have listeners registered
55-
expect(el1.firstChild.addEventListener).toHaveBeenCalledTimes(3)
56-
expect(el2.firstChild.addEventListener).toHaveBeenCalledTimes(3)
57-
expect(el3.firstChild.addEventListener).toHaveBeenCalledTimes(3)
60+
boundEls.push(el1, el2, el3)
61+
62+
// All elements should have the expected event listeners registered
63+
for (const el of [el1, el2, el3]) {
64+
expect(el.firstChild.addEventListener).toHaveBeenCalledWith(
65+
'dragover',
66+
expect.any(Function),
67+
)
68+
expect(el.firstChild.addEventListener).toHaveBeenCalledWith(
69+
'dragleave',
70+
expect.any(Function),
71+
)
72+
expect(el.firstChild.addEventListener).toHaveBeenCalledWith(
73+
'drop',
74+
expect.any(Function),
75+
)
76+
}
5877
})
5978

60-
it('updates the correct instance on update', () => {
61-
const updateSpy = vi.spyOn(DroppableMailbox.prototype, 'update')
62-
79+
it('updates the correct instance on componentUpdated', () => {
6380
const el1 = createMockEl('el1')
6481
const el2 = createMockEl('el2')
6582

6683
bind(el1, createBinding({ mailboxId: 1, isValidDropTarget: true }))
6784
bind(el2, createBinding({ mailboxId: 2, isValidDropTarget: true }))
68-
69-
componentUpdated(el1, createBinding({ mailboxId: 1, isValidDropTarget: false }))
70-
71-
// update() should have been called on the instance bound to el1
72-
expect(updateSpy).toHaveBeenCalledTimes(1)
73-
expect(updateSpy.mock.instances[0].el).toBe(el1)
74-
75-
updateSpy.mockRestore()
85+
boundEls.push(el1, el2)
86+
87+
// Update el1 to disable drop target
88+
componentUpdated(
89+
el1,
90+
createBinding({ mailboxId: 1, isValidDropTarget: false }),
91+
)
92+
93+
// Clear setAttribute calls from bind so we only see drag-start effects
94+
el1.setAttribute.mockClear()
95+
el2.setAttribute.mockClear()
96+
97+
// Trigger drag-start on all instances via the event bus
98+
dragEventBus.emit('drag-start', { accountId: 1, mailboxId: 99 })
99+
100+
// el1 should be disabled because its isValidDropTarget was set to false
101+
expect(el1.setAttribute).toHaveBeenCalledWith(
102+
'droppable-mailbox',
103+
'disabled',
104+
)
105+
106+
// el2 should NOT be disabled because its options were not changed
107+
expect(el2.setAttribute).not.toHaveBeenCalledWith(
108+
'droppable-mailbox',
109+
'disabled',
110+
)
76111
})
77112

78113
it('removes listeners and instance on unbind', () => {
@@ -81,11 +116,23 @@ describe('DroppableMailboxDirective', () => {
81116

82117
bind(el1, createBinding({ mailboxId: 1 }))
83118
bind(el2, createBinding({ mailboxId: 2 }))
119+
boundEls.push(el2)
84120

85121
unbind(el1)
86122

87-
// el1 should have had removeEventListener called
88-
expect(el1.firstChild.removeEventListener).toHaveBeenCalledTimes(3)
123+
// el1 should have had removeEventListener called for each event
124+
expect(el1.firstChild.removeEventListener).toHaveBeenCalledWith(
125+
'dragover',
126+
expect.any(Function),
127+
)
128+
expect(el1.firstChild.removeEventListener).toHaveBeenCalledWith(
129+
'dragleave',
130+
expect.any(Function),
131+
)
132+
expect(el1.firstChild.removeEventListener).toHaveBeenCalledWith(
133+
'drop',
134+
expect.any(Function),
135+
)
89136

90137
// el2 should NOT have had removeEventListener called
91138
expect(el2.firstChild.removeEventListener).not.toHaveBeenCalled()

0 commit comments

Comments
 (0)