Skip to content

Add more tests #30

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.

Sign up for GitHub

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

Merged
satnam72 merged 2 commits into from
Aug 28, 2025
Merged

Add more tests #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
68c0ee9
fix(package): update version in package.json
satnam72 Aug 28, 2025
ebdda48
test: add tests for update and addDraggable methods; update implement…
satnam72 Aug 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "allonsh",
"version": "0.1.0",
"version": "0.2.0-beta",
"description": "Lightweight, dependency-free drag-and-drop with precise native target detection and easy customization.",
"main": "src/allonsh.js",
"directories": {
Expand Down
6 changes: 6 additions & 0 deletions src/allonsh.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ class Allonsh {
console.warn(
`Allonsh Warning: No draggable elements found with selector '.${draggableSelector}'.`
);
} else {
this.draggableElements.forEach((element) => {
if (!element.classList.contains(CSS_CLASSES.DRAGGABLE)) {
element.classList.add(CSS_CLASSES.DRAGGABLE);
}
});
}

if (dropzoneSelector) {
Expand Down
36 changes: 36 additions & 0 deletions test/allonsh.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,4 +164,40 @@ describe('Allonsh', () => {

expect(dropHandler).toHaveBeenCalled();
});

it('should not duplicate event listeners on update', () => {
const el = allonsh.draggableElements[0];
const addSpy = vi.spyOn(el, 'addEventListener');

allonsh.update({ restrictToDropzones: true });
allonsh.update({ enableStacking: false });
allonsh.update({ useGhostEffect: false });

const mousedownAdds = addSpy.mock.calls.filter(
([event]) => event === 'mousedown'
);
const touchstartAdds = addSpy.mock.calls.filter(
([event]) => event === 'touchstart'
);

expect(mousedownAdds.length).toBeLessThanOrEqual(1);
expect(touchstartAdds.length).toBeLessThanOrEqual(1);

addSpy.mockRestore();
});
Comment on lines +168 to +187
Copy link

@coderabbitai coderabbitai bot Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Test is a false negative; update() currently queries with undefined selector → no adds observed. Tighten the assertion and ensure the selector path is exercised.

Pass the selector (same value) and require zero new adds to prove no duplication.

Apply:

-  it('should not duplicate event listeners on update', () => {
+  it('should not duplicate event listeners on update', () => {
     const el = allonsh.draggableElements[0];
     const addSpy = vi.spyOn(el, 'addEventListener');
 
-    allonsh.update({ restrictToDropzones: true });
-    allonsh.update({ enableStacking: false });
-    allonsh.update({ useGhostEffect: false });
+    // Provide the same selector to exercise the re-selection path without rebinding
+    allonsh.update({ restrictToDropzones: true, draggableSelector: 'draggable' });
+    allonsh.update({ enableStacking: false, draggableSelector: 'draggable' });
+    allonsh.update({ useGhostEffect: false, draggableSelector: 'draggable' });
 
     const mousedownAdds = addSpy.mock.calls.filter(
       ([event]) => event === 'mousedown'
     );
     const touchstartAdds = addSpy.mock.calls.filter(
       ([event]) => event === 'touchstart'
     );
 
-    expect(mousedownAdds.length).toBeLessThanOrEqual(1);
-    expect(touchstartAdds.length).toBeLessThanOrEqual(1);
+    expect(mousedownAdds.length).toBe(0);
+    expect(touchstartAdds.length).toBe(0);
 
     addSpy.mockRestore();
   });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('should not duplicate event listeners on update', () => {
const el = allonsh.draggableElements[0];
const addSpy = vi.spyOn(el, 'addEventListener');
allonsh.update({ restrictToDropzones: true });
allonsh.update({ enableStacking: false });
allonsh.update({ useGhostEffect: false });
const mousedownAdds = addSpy.mock.calls.filter(
([event]) => event === 'mousedown'
);
const touchstartAdds = addSpy.mock.calls.filter(
([event]) => event === 'touchstart'
);
expect(mousedownAdds.length).toBeLessThanOrEqual(1);
expect(touchstartAdds.length).toBeLessThanOrEqual(1);
addSpy.mockRestore();
});
it('should not duplicate event listeners on update', () => {
const el = allonsh.draggableElements[0];
const addSpy = vi.spyOn(el, 'addEventListener');
// Provide the same selector to exercise the re-selection path without rebinding
allonsh.update({ restrictToDropzones: true, draggableSelector: 'draggable' });
allonsh.update({ enableStacking: false, draggableSelector: 'draggable' });
allonsh.update({ useGhostEffect: false, draggableSelector: 'draggable' });
const mousedownAdds = addSpy.mock.calls.filter(
([event]) => event === 'mousedown'
);
const touchstartAdds = addSpy.mock.calls.filter(
([event]) => event === 'touchstart'
);
expect(mousedownAdds.length).toBe(0);
expect(touchstartAdds.length).toBe(0);
addSpy.mockRestore();
});
🤖 Prompt for AI Agents 
test/allonsh.test.js around lines 168 to 187: the test currently observes no
listener additions because update() is being called without the selector so the
internal query path is not exercised; modify the three allonsh.update calls to
pass the same selector used to initialize draggableElements (so the query runs),
then tighten the assertions to require zero new 'mousedown' and 'touchstart'
addEventListener calls (expect(...length).toBe(0)); keep the spy setup and
mockRestore as is.


it('should preserve existing draggables and increment count by 1 when addDraggable() is called', () => {
const initialCount = allonsh.draggableElements.length;

const newEl = document.createElement('div');
newEl.className = 'draggable new';
document.querySelector('.play-area').appendChild(newEl);

allonsh.addDraggable(newEl);

const finalCount = allonsh.draggableElements.length;

expect(finalCount).toBe(initialCount + 1);
expect([...allonsh.draggableElements]).toContain(newEl);
});
});