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.

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

Playwright tests fail when choosing option in select beyond initial viewport #7516

Open
jabrks opened this issue Dec 12, 2024 · 9 comments · May be fixed by #7541
Open

Playwright tests fail when choosing option in select beyond initial viewport #7516

jabrks opened this issue Dec 12, 2024 · 9 comments · May be fixed by #7541

Comments

@jabrks
Copy link

jabrks commented Dec 12, 2024

Provide a general summary of the issue here

We've encountered an issue after upgrading from v1.4.1 of the library whereby Playwright tests that try to open a select component found below the fold and choose an option will fail

🤔 Expected Behavior?

The test should pass successfully

😯 Current Behavior

The test times out whilst trying to locate the option

💁 Possible Solution

No response

🔦 Context

No response

🖥️ Steps to Reproduce

https://github.com/jabrks/react-aria-reproduction

There's a GitHub action that runs on the main branch in the repo that demonstrates the test failure, or you can clone the code and run the tests locally yourself with npm test

Version

react-aria-components 1.5.0

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

macOS

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

@snowystinger
Copy link
Member

Probably need to scroll to the select before opening it?
Or does playwright handle that?

@jabrks
Copy link
Author

jabrks commented Dec 13, 2024

I believe Playwright handles it. The test was working as expected with RAC v1.4.1 and below so I had presumed a regression somewhere was to blame rather than anything to do with Playwright itself

@jabrks
Copy link
Author

jabrks commented Dec 13, 2024

I have cherry-picked the commits in the release to try and narrow down the cause and it seems as though the issue was introduced as a result of the changes to @react-aria/overlays in #7368?

@snowystinger
Copy link
Member

Awesome, thank you for doing that. It's super helpful.
Now we'll just need to determine what in there was the culprit, probably best to start with the changes in @react-aria/overlays. Theoretically, there should have been no behavioral changes, just strict mode fixes. So, we should be able to undo changes in each file individually to track down the cause.

If you don't have time, don't worry. I'm just outlining what I would do for whoever comes along next.

@jabrks
Copy link
Author

jabrks commented Dec 13, 2024

Changing this undefined back to null fixes it for me

onClose: isNonModal ? state.close : undefined

I am not entirely clear yet as to why that is the case...

@jabrks
Copy link
Author

jabrks commented Dec 13, 2024

I think I can see the problem. There's a condition in useCloseOnScroll for if onClose is null which is no longer being met, resulting in this being called where it previously wasn't

if (!isOpen || onClose === null) {
return;
}

@jabrks
Copy link
Author

jabrks commented Dec 19, 2024

If I were to submit a PR for this, is there a particular approach to fixing it that you'd like me to take?

@snowystinger
Copy link
Member

I think it'd make the most sense to change the types on useCloseOnScroll to include null. It looks like the different behavior was added on purpose e4ab489

Then go back and change usePopover to pass null again.

I'm not sure why TS didn't mark it as dead code.

The other option would be to remove that check entirely. It doesn't fail anything to do that, but I don't know what other implications there are, so it'd need to be tested in storybook. Specifically on an iPad according to that commit/PR.

@jabrks jabrks linked a pull request Dec 20, 2024 that will close this issue
5 tasks
@krystian-jedrzejowski-zazoon

I believe Playwright handles it. The test was working as expected with RAC v1.4.1 and below so I had presumed a regression somewhere was to blame rather than anything to do with Playwright itself

I have same behaviour. We have upgraded "react-aria-components": "1.4.1" to ver "1.5.0" and playwright tests became flaky. I have noticed that when I have dropdown component open for selection Playwright do the scroll to find an option and it causes the dropdown to close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants