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

fix: resizing window causes 'ResizeObserver loop completed with undelivered notifications' error #7742

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

reidbarber
Copy link
Member

Closes #7718

The error shown is likely due to how DOM changes triggered by the resize observer cause further resize events in the same frame. We can mitigate this by waiting until the next frame to apply the updates. This one may need a decent amount of manual regression testing.

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

  1. Open the RAC -> Popover Example story in it's own tab
  2. Open the console and resize your window rapidly to cause the popover position to be updated
  3. Verify that there is no ResizeObserver loop completed with undelivered notifications error in the console

🧢 Your Project:

@rspbot
Copy link

rspbot commented Feb 7, 2025

@snowystinger
Copy link
Member

I did this a long time ago
#2891

Which we had to revert due to #4204

We'll have to check if the issue that caused us to revert it happens in this PR as well. There isn't a lot of info on what it was, might be good to go back and check some of our testing sheets if you can.

@reidbarber
Copy link
Member Author

reidbarber commented Feb 10, 2025

@snowystinger

Looked back and looks like tooltip positions were broken in chromatic, which led to #4202 (comment), which we reverted because the original bug was reintroduced.

@reidbarber
Copy link
Member Author

@rspbot
Copy link

rspbot commented Feb 11, 2025

@snowystinger
Copy link
Member

snowystinger commented Feb 11, 2025

@snowystinger not seeing anything noteworthy: https://www.chromatic.com/build?appId=5f0dd5ad2b5fc10022a2e320&number=900

https://www.chromatic.com/test?appId=5f0dd5ad2b5fc10022a2e320&id=67aa8fee55d156ca20f02823
this one is new, it's not a flaky test

That said, it looks like the chromatic snapshot has been wrong, it shouldn't have been stacked vertically all this time. Let's see if we can find out more about what has happened there

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

Successfully merging this pull request may close these issues.

ResizeObserver loop completed with undelivered notifications
3 participants