-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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.
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
The type command should not click on focused elements inside of shadow roots. #26198
Comments
Hi, thanks for the minimal reproduction. Just to make it even more minimal, it looks like we can remove some code - can you confirm this still represents your use case? https://github.com/kgroat/cypress-type-repro/pull/1/files. I also made a minimal non web components/shadow example, to see what happens - it does indeed pass: <!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Document</title>
</head>
<body>
<input id="input" type='text' onclick="handleClick()" />
<div id="div" tabindex="-1" class="popup">
I am a popup
</div>
<script id="no-shadow-dom">
const $ = sel => document.querySelector(sel)
const $input = $("#input")
const $div = $("#div")
document.addEventListener('click', (e) => {
console.log('click')
handleDocClick(e)
});
function handleDocClick(e) {
console.log(e.target)
if (e.target === $input || e.target === $div) return
open = false;
$div.focus();
}
let open = false
async function handleClick() {
if (open) return;
console.log('set')
open = true;
$div.setAttribute('tabindex', 0)
$div.classList.add('open')
$div.focus();
}
</script>
<style>
.popup {
display: none;
}
.open {
display: block;
}
</style>
</body>
</html> It looks like there was some somewhat related work in #7847. I'd say the best option here would be "Either check inside of shadow roots for the focused element". |
Your PR does take out some unnecessary code; I'll go ahead and merge it. That said, the test in there I think we should keep the Also, yes, the code above seems to pass the test, which is why I believe this has to do with shadow dom. Checking inside the shadow roots would be my preferred option as well. I think I've tracked a solution down.
Should I open a PR with my proposed solution? I've verified it using |
Oh nice, I was about to ask if you wanted to make a PR. If you think you've got a fix, go ahead and make a PR. I'll need to kick off the CI since you are a third party, so you could add me as a reviewer. Lmk if you need any other help navigating the code base, adding a reproduction, or anything else. Thanks! |
I'm not sure how to invite you to be a reviewer, but you can see the PR above. I've marked it as a draft while I add a test for the scenario. I see that there are only e2e tests in the driver package. How would I run those in order to verify my test works? I tried |
Figured out that I could just |
You can run You can do You can also do |
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Current behavior
When you attempt to
.type()
inside an element that is inside a shadow root, it will always perform the.click()
even if the element has focus.This is even an issue if you have
includeShadowDom
set totrue
.Desired behavior
Either check inside of shadow roots for the focused element, or allow the
.click()
behavior to be turned off using an option inside the.type()
command.Test code to reproduce
https://github.com/kgroat/cypress-type-repro
Cypress Version
12.8.1
Node version
16.16.0
Operating System
MacOS 13.2.1
Debug Logs
No response
Other
No response
The text was updated successfully, but these errors were encountered: