Skip to content
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

test: update animation click test #1053

Merged
merged 3 commits into from
Feb 18, 2020
Merged

test: update animation click test #1053

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f499c32
test: update animation click test
dgozman Feb 18, 2020
ef3ceb7
Update test/click.spec.js
dgozman Feb 18, 2020
5335e52
Update test/click.spec.js
dgozman Feb 18, 2020
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
68 changes: 42 additions & 26 deletions test/click.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,35 +403,51 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
await page.click('button');
expect(await page.evaluate('window.clicked')).toBe(true);
});
xit('should click on an animated button', async({page}) => {
const buttonSize = 50;
xit('should fail to click a button animated via CSS animations and setInterval', async({page}) => {
// This test has a setInterval that consistently animates a button.
// It checks that we detect the button to be continuously animating, and never try to click it.
// This test exposes two issues:
// - Chromium headless does not issue rafs between first and second animateLeft() calls.
// - Chromium and WebKit keep element bounds the same when for 2 frames when changing left to a new value.
const buttonSize = 10;
const containerWidth = 500;
const transition = 500;
const transition = 100;
await page.setContent(`
<html>
<body>
<div style="border: 1px solid black; height: 50px; overflow: auto; width: ${containerWidth}px;">
<button id="button" style="height: ${buttonSize}px; width: ${buttonSize}px; transition: left ${transition}ms linear 0s; left: 0; position: relative" onClick="window.clicked++">hi</button>
</div>
</body>
<script>
const animateLeft = () => {
const button = document.querySelector('#button');
document.querySelector('#button').style.left = button.style.left === '0px' ? '${containerWidth - buttonSize}px' : '0px';
};
window.clicked = 0;
window.setTimeout(animateLeft, 0);
window.setInterval(animateLeft, ${transition});
</script>
</html>
<html>
<body>
<div style="border: 1px solid black; height: 50px; overflow: auto; width: ${containerWidth}px;">
<button style="border: none; height: ${buttonSize}px; width: ${buttonSize}px; transition: left ${transition}ms linear 0s; left: 0; position: relative" onClick="window.clicked++"></button>
</div>
</body>
<script>
window.atLeft = true;
const animateLeft = () => {
const button = document.querySelector('button');
button.style.left = window.atLeft ? '${containerWidth - buttonSize}px' : '0px';
console.log('set ' + button.style.left);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: unneeded console log

window.atLeft = !window.atLeft;
};
window.clicked = 0;
const dump = () => {
const button = document.querySelector('button');
const clientRect = button.getBoundingClientRect();
const rect = { x: clientRect.top, y: clientRect.left, width: clientRect.width, height: clientRect.height };
requestAnimationFrame(dump);
};
dump();
</script>
</html>
`);
await page.click('button');
expect(await page.evaluate('window.clicked')).toBe(1);
expect(await page.evaluate('document.querySelector("#button").style.left')).toBe(`${containerWidth - buttonSize}px`);
await new Promise(resolve => setTimeout(resolve, 500));
await page.click('button');
expect(await page.evaluate('window.clicked')).toBe(2);
expect(await page.evaluate('document.querySelector("#button").style.left')).toBe('0px');
await page.evaluate(transition => {
window.setInterval(animateLeft, transition);
Copy link
Contributor

Choose a reason for hiding this comment

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

If we care specifically about the rafs, then this should probably be animated on raf right?
I'm not sure what we are testing, but if I was reviewing this code for a website I would say this is broken. You should always animate on raf, not setInterval.

animateLeft();
}, transition);
const error1 = await page.click('button', { timeout: 250 }).catch(e => e);
expect(await page.evaluate('window.clicked')).toBe(0);
expect(error1.message).toContain('timeout 250ms exceeded');
const error2 = await page.click('button', { timeout: 250 }).catch(e => e);
expect(await page.evaluate('window.clicked')).toBe(0);
expect(error2.message).toContain('timeout 250ms exceeded');
});
});

Expand Down