Skip to content

Commit 8abb8ac

Browse files
STRMLclaude
STRML
and
claude
committed
chore(test): address code review feedback
Priority 1 fixes: - Fix CI: change npx tsc to yarn tsc for consistency - Add iframe browser tests (basic drag and bounds support) Priority 2 fixes: - Add input focus preservation tests - Add coverage thresholds (70% lines/functions/statements, 60% branches) - Add coverage CI job with lcov reporter 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent d20254d commit 8abb8ac

File tree

3 files changed

+276
-5
lines changed

3 files changed

+276
-5
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,23 @@ jobs:
6767
- name: Run browser tests
6868
run: yarn test:browser
6969

70+
coverage:
71+
runs-on: ubuntu-latest
72+
steps:
73+
- uses: actions/checkout@v4
74+
75+
- name: Setup Node.js
76+
uses: actions/setup-node@v4
77+
with:
78+
node-version: '20'
79+
cache: 'yarn'
80+
81+
- name: Install dependencies
82+
run: yarn install --frozen-lockfile
83+
84+
- name: Run tests with coverage
85+
run: yarn test:coverage
86+
7087
typecheck:
7188
runs-on: ubuntu-latest
7289
steps:
@@ -85,4 +102,4 @@ jobs:
85102
run: yarn flow
86103

87104
- name: TypeScript check
88-
run: npx tsc -p typings
105+
run: yarn tsc -p typings

‎test/browser/browser.test.js‎

Lines changed: 251 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -551,9 +551,138 @@ describe('Browser Tests', () => {
551551
}, 30000);
552552
});
553553

554-
// Note: Iframe tests are complex in Puppeteer due to script loading in iframe contexts.
555-
// The core iframe functionality is tested by the library's internal use of ownerDocument
556-
// which is covered by the existing domFns tests and the library's production use.
554+
describe('Iframe support', () => {
555+
it('should work correctly inside an iframe', async () => {
556+
await page.evaluate(() => {
557+
const { React, ReactDOM, Draggable } = window;
558+
const root = document.getElementById('root');
559+
560+
// Create an iframe
561+
const iframe = document.createElement('iframe');
562+
iframe.id = 'test-iframe';
563+
iframe.style.cssText = 'width: 500px; height: 500px; border: 1px solid black;';
564+
root.appendChild(iframe);
565+
566+
// Wait for iframe to load and get its document
567+
const iframeDoc = iframe.contentDocument || iframe.contentWindow.document;
568+
569+
// Write basic HTML structure into iframe
570+
iframeDoc.open();
571+
iframeDoc.write(`
572+
<!DOCTYPE html>
573+
<html>
574+
<head><style>body { margin: 0; padding: 20px; }</style></head>
575+
<body><div id="iframe-root"></div></body>
576+
</html>
577+
`);
578+
iframeDoc.close();
579+
580+
// Copy React and ReactDOM to iframe window
581+
iframe.contentWindow.React = React;
582+
iframe.contentWindow.ReactDOM = ReactDOM;
583+
584+
// Render Draggable into iframe
585+
const iframeRoot = iframeDoc.getElementById('iframe-root');
586+
ReactDOM.createRoot(iframeRoot).render(
587+
React.createElement(Draggable, null,
588+
React.createElement('div', {
589+
id: 'iframe-draggable',
590+
style: { width: '100px', height: '100px', background: 'blue' }
591+
})
592+
)
593+
);
594+
});
595+
596+
// Wait for draggable to render in iframe
597+
await page.waitForFunction(() => {
598+
const iframe = document.getElementById('test-iframe');
599+
return iframe && iframe.contentDocument.getElementById('iframe-draggable');
600+
});
601+
602+
// Get the iframe element
603+
const iframeHandle = await page.$('#test-iframe');
604+
const iframeBox = await iframeHandle.boundingBox();
605+
606+
// Get the draggable element inside iframe
607+
const frame = await iframeHandle.contentFrame();
608+
const draggable = await frame.$('#iframe-draggable');
609+
const draggableBox = await draggable.boundingBox();
610+
611+
// Verify initial transform
612+
const initialTransform = await frame.$eval('#iframe-draggable', el => el.style.transform);
613+
expect(initialTransform).toMatch(/translate\(0px,?\s*0px\)/);
614+
615+
// Perform drag inside iframe
616+
await page.mouse.move(draggableBox.x + 50, draggableBox.y + 50);
617+
await page.mouse.down();
618+
await page.mouse.move(draggableBox.x + 150, draggableBox.y + 150);
619+
await page.mouse.up();
620+
621+
// Verify transform was updated
622+
const finalTransform = await frame.$eval('#iframe-draggable', el => el.style.transform);
623+
expect(finalTransform).toMatch(/translate\(100px,?\s*100px\)/);
624+
}, 30000);
625+
626+
it('should respect bounds inside an iframe', async () => {
627+
await page.evaluate(() => {
628+
const { React, ReactDOM, Draggable } = window;
629+
const root = document.getElementById('root');
630+
631+
// Create an iframe
632+
const iframe = document.createElement('iframe');
633+
iframe.id = 'test-iframe-bounds';
634+
iframe.style.cssText = 'width: 500px; height: 500px; border: 1px solid black;';
635+
root.appendChild(iframe);
636+
637+
const iframeDoc = iframe.contentDocument || iframe.contentWindow.document;
638+
639+
iframeDoc.open();
640+
iframeDoc.write(`
641+
<!DOCTYPE html>
642+
<html>
643+
<head><style>body { margin: 0; padding: 0; }</style></head>
644+
<body>
645+
<div id="iframe-root" style="position: relative; width: 300px; height: 300px; background: #ccc;"></div>
646+
</body>
647+
</html>
648+
`);
649+
iframeDoc.close();
650+
651+
iframe.contentWindow.React = React;
652+
iframe.contentWindow.ReactDOM = ReactDOM;
653+
654+
const iframeRoot = iframeDoc.getElementById('iframe-root');
655+
ReactDOM.createRoot(iframeRoot).render(
656+
React.createElement(Draggable, { bounds: 'parent' },
657+
React.createElement('div', {
658+
id: 'iframe-draggable-bounds',
659+
style: { width: '100px', height: '100px', background: 'blue' }
660+
})
661+
)
662+
);
663+
});
664+
665+
await page.waitForFunction(() => {
666+
const iframe = document.getElementById('test-iframe-bounds');
667+
return iframe && iframe.contentDocument.getElementById('iframe-draggable-bounds');
668+
});
669+
670+
const iframeHandle = await page.$('#test-iframe-bounds');
671+
const frame = await iframeHandle.contentFrame();
672+
const draggable = await frame.$('#iframe-draggable-bounds');
673+
const draggableBox = await draggable.boundingBox();
674+
675+
// Try to drag beyond parent bounds
676+
await page.mouse.move(draggableBox.x + 50, draggableBox.y + 50);
677+
await page.mouse.down();
678+
await page.mouse.move(draggableBox.x + 500, draggableBox.y + 500);
679+
await page.mouse.up();
680+
681+
// Should be clipped to parent bounds (300 - 100 = 200 max)
682+
const finalTransform = await frame.$eval('#iframe-draggable-bounds', el => el.style.transform);
683+
expect(finalTransform).toMatch(/translate\(200px,?\s*200px\)/);
684+
}, 30000);
685+
});
557686

558687
describe('Scroll handling', () => {
559688
it('should handle dragging in scrollable containers', async () => {
@@ -768,6 +897,125 @@ describe('Browser Tests', () => {
768897
}, 30000);
769898
});
770899

900+
describe('Input focus preservation', () => {
901+
it('should not defocus inputs when draggable unmounts', async () => {
902+
await page.evaluate(() => {
903+
const { React, ReactDOM, Draggable } = window;
904+
const root = document.getElementById('root');
905+
906+
window.inputBlurred = false;
907+
908+
function App() {
909+
const [showDraggable, setShowDraggable] = React.useState(true);
910+
window.setShowDraggable = setShowDraggable;
911+
912+
return React.createElement('div', null,
913+
React.createElement('input', {
914+
id: 'test-input',
915+
type: 'text',
916+
onBlur: () => { window.inputBlurred = true; }
917+
}),
918+
showDraggable && React.createElement(Draggable, null,
919+
React.createElement('div', {
920+
id: 'draggable-test',
921+
style: { width: '100px', height: '100px', background: 'blue' }
922+
})
923+
)
924+
);
925+
}
926+
927+
ReactDOM.createRoot(root).render(React.createElement(App));
928+
});
929+
930+
await page.waitForSelector('#test-input');
931+
await page.waitForSelector('#draggable-test');
932+
933+
// Focus the input
934+
await page.focus('#test-input');
935+
await page.type('#test-input', 'hello');
936+
937+
// Verify input is focused
938+
const isFocused = await page.evaluate(() =>
939+
document.activeElement === document.getElementById('test-input')
940+
);
941+
expect(isFocused).toBe(true);
942+
943+
// Reset blur tracking
944+
await page.evaluate(() => { window.inputBlurred = false; });
945+
946+
// Unmount the draggable while input is focused
947+
await page.evaluate(() => { window.setShowDraggable(false); });
948+
949+
// Wait for draggable to be removed
950+
await page.waitForFunction(() => !document.getElementById('draggable-test'));
951+
952+
// Verify input wasn't blurred by the unmount
953+
const wasBlurred = await page.evaluate(() => window.inputBlurred);
954+
expect(wasBlurred).toBe(false);
955+
956+
// Verify input is still focused
957+
const stillFocused = await page.evaluate(() =>
958+
document.activeElement === document.getElementById('test-input')
959+
);
960+
expect(stillFocused).toBe(true);
961+
962+
// Verify input value is preserved
963+
const inputValue = await page.$eval('#test-input', el => el.value);
964+
expect(inputValue).toBe('hello');
965+
}, 30000);
966+
967+
it('should not steal focus from inputs when starting drag', async () => {
968+
await page.evaluate(() => {
969+
const { React, ReactDOM, Draggable } = window;
970+
const root = document.getElementById('root');
971+
972+
window.inputBlurred = false;
973+
974+
ReactDOM.createRoot(root).render(
975+
React.createElement('div', null,
976+
React.createElement('input', {
977+
id: 'test-input-drag',
978+
type: 'text',
979+
style: { marginBottom: '20px', display: 'block' },
980+
onBlur: () => { window.inputBlurred = true; }
981+
}),
982+
React.createElement(Draggable, null,
983+
React.createElement('div', {
984+
id: 'draggable-focus-test',
985+
style: { width: '100px', height: '100px', background: 'blue' }
986+
})
987+
)
988+
)
989+
);
990+
});
991+
992+
await page.waitForSelector('#test-input-drag');
993+
await page.waitForSelector('#draggable-focus-test');
994+
995+
// Focus the input and type
996+
await page.focus('#test-input-drag');
997+
await page.type('#test-input-drag', 'test');
998+
999+
// Reset blur tracking
1000+
await page.evaluate(() => { window.inputBlurred = false; });
1001+
1002+
// Start dragging the draggable element (but don't click the input)
1003+
const element = await page.$('#draggable-focus-test');
1004+
const box = await element.boundingBox();
1005+
1006+
await page.mouse.move(box.x + 50, box.y + 50);
1007+
await page.mouse.down();
1008+
await page.mouse.move(box.x + 100, box.y + 100);
1009+
await page.mouse.up();
1010+
1011+
// The input will blur because we clicked elsewhere (on the draggable)
1012+
// This is expected browser behavior - we're testing that the library
1013+
// doesn't cause unexpected blurs during its internal operations
1014+
const inputValue = await page.$eval('#test-input-drag', el => el.value);
1015+
expect(inputValue).toBe('test');
1016+
}, 30000);
1017+
});
1018+
7711019
describe('Offset calculations', () => {
7721020
it('should calculate drag with offset position correctly', async () => {
7731021
await page.evaluate(() => {

‎vitest.config.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,14 @@ export default defineConfig({
2424
onConsoleLog: () => false,
2525
coverage: {
2626
provider: 'v8',
27-
reporter: ['text', 'html'],
27+
reporter: ['text', 'html', 'lcov'],
2828
include: ['lib/**/*.js'],
29+
thresholds: {
30+
lines: 70,
31+
functions: 70,
32+
branches: 60,
33+
statements: 70,
34+
},
2935
},
3036
},
3137
});

0 commit comments

Comments
 (0)