patternfly / patternfly-elements

PatternFly Elements. A set of community-created web components based on PatternFly design.
https://patternflyelements.org/
MIT License
379 stars 96 forks source link

fix(core): overflow controller resize loop #2855

Closed zeroedin closed 2 months ago

zeroedin commented 2 months ago

Downstream issue:

> on small screen > reversed right to left language overflow actions > previousTab should be disabled Error: ResizeObserver loop completed with undelivered notifications. (http://localhost:8003/?wtr-session-id=E_xerQsq1EQq2yny1h-Ga:0)

If you want to prevent these errors, the solution depends on what your intended effect is. If you actually intend to have an infinite loop, you just need to defer the resizing code in your ResizeObserver callback to after the browser repaints. You can put it into a requestAnimationFrame callback.

Documentation Reference

What I did

  1. Await browser repaint before triggering overflow side effects

Testing Instructions

1.

Notes to Reviewers

1.

changeset-bot[bot] commented 2 months ago

🦋 Changeset detected

Latest commit: ac185357697e7b718d04b896eae2e7a5e3e4826b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | -------------------- | ----- | | @patternfly/pfe-core | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

netlify[bot] commented 2 months ago

Deploy Preview for patternfly-elements ready!

Name Link
Latest commit 288f7b641523b0428307b8d93e8cf313f8f557ff
Deploy Preview https://deploy-preview-2855--patternfly-elements.netlify.app/

To edit notification comments on pull requests, go to your Netlify site settings.

github-actions[bot] commented 2 months ago

✅ Commitlint tests passed!

More Info ```json { "valid": true, "errors": [], "warnings": [], "input": "fix(core): overflow controller resize loop" } ```
zeroedin commented 2 months ago

@bennypowers adding await aTimeout(50); allows the pf-tabs tests to complete. That said I'm not convinced that just adding a timeout to the test is the correct way to handle red/green here. Another way to do it is await 3x nextFrame.

beforeEach(nextFrame);
beforeEach(nextFrame);
beforeEach(nextFrame);
beforeEach(updateComplete);

I can almost understand the need for nextFrame better here as we are now adding a tick given the overflow controller implementing the requestAnimationFrame. Thoughts?